[DBTNG + simpletest] missing db_select()->temporary()

It is not clear why you create the temporary table in a separate request. The phpdoc does not explain the 'why'.

Test failure : #335122: Test clean HEAD after every commit

Apropos of the discussion over in #349500: db_query_temporary() should generate a table name, we shouldn't allow callers to specify their own table name. I'm actually thinking it would be better here to have an alternate execute method, executeTemporary() or some such, that takes the built query and makes a temp table out of it and returns the table name it generated, just like db_query_temporary() will after that patch lands. That means this patch is blocked on that one. hswong, can you reroll with that approach as soon as the other patch goes in? Thanks.

Sigh. I guess this didn't happen. Sadness.

Downgrading all D8 criticals to major per http://drupal.org/node/45111

Subscribing.
While probably not used very often, this is one of the (few) holes in the API.

Picking this up 4 years later...

Since this involves a change to an interface, I'm guessing it might be too late for 8.0?

1. +++ b/core/includes/database.inc
   @@ -321,6 +321,8 @@ function db_query_range($query, $from, $count, array $args = array(), array $opt
   + * @deprecated in Drupal 8.0.x, will be removed before 8.1.0.
   

   Why did you add this? Seems no other database function is deprecated, why should this be? (Or, in any case, that's a different issue.)

2. +++ b/core/lib/Drupal/Core/Database/Query/Select.php
   @@ -572,6 +572,20 @@ public function execute() {
   +    // Note that validation routines in preExecute() may throw exceptions instead.
   

   +++ b/core/lib/Drupal/Core/Database/Query/SelectInterface.php
   @@ -489,6 +489,14 @@ public function preExecute(SelectInterface $query = NULL);
   +   * Executes a SELECT query string and saves the result set to a temporary table.
   

   Overlong comment lines.

3. +++ b/core/lib/Drupal/Core/Database/Query/SelectInterface.php
   @@ -489,6 +489,14 @@ public function preExecute(SelectInterface $query = NULL);
   +   * @return string
   +   *   The name of the temporary table.
   

   Should also list NULL for the error case. Should also list the exception that could be thrown, but it seems that isn't the case anywhere else in this interface either, anyways.

Amended patch attached. Otherwise, this looks great, thanks a lot for posting!
I do hope we can still get this in, it seems innocent enough and has been broken for two major versions now. But of course, that's not any argument here …

The db_query_temporary() function isn't used at all by core, and because it isn't just a wrapper for this new method, then two different code paths need to be maintained. Seemed reasonable to deprecate and eventually remove.

monkey: Please include an interdiff, especially when fixing your own review.

I'm fine with this, including in 8.0. Up to catch if it gets bumped to 8.1.

(I would like to mark all of the db_*() functions deprecated but we can't remove them in 8.x now, sadly.)

We can deprecate stuff for 9.x - this issue should deprecate db_query_temporary().

This issue does not fall into the list of unfrozen changes in https://www.drupal.org/core/beta-changes. This issue looks like a good candidate for 8.1

monkey: Please include an interdiff, especially when fixing your own review.

OK, sorry. Here is an interdiff for my previous patch, and a new patch with the @deprecated added again, along with an interdiff.
However, in my opinion, we should mark either all or none of these as deprecated, and in a separate issue. This has little to do with the issue at hand.

Active again, I guess?

Re-roll and small fix.

db_query_temporary() is already marked as deprecated (see #1894396: Mark db_*() wrappers in database.inc as @deprecated for 9.x).

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Tagging for IS update as after so many years I know things are different for D10 and imagine the fix (if any) will be different.

Moving to PNMI if someone could answer if this is still an issue for 10.1 (apologies for not knowing myself just trying to help with the queue)

If still a valid feature request please reopen updating issue summary per #39

Thanks!