dbtng drivers are not able to have their own condition classes

Marking this as major - since #1 is clearly discouraged.

I have no idea why you need committer feedback when this hasn't even been discussed yet. Please call me in when things are deadlocked and there's no way forward. I can't commit when I'm feedbacking.

The pattern we are trying to match is simple enough that it feels like #2 should be feasible in at least osme cases.

I'd still love to see a query tag/alter for everything, not just selects.

You can double up your subselects to avoid the problem http://www.xaprb.com/blog/2006/04/30/how-to-optimize-subqueries-and-join...

Where's the fix? I don't see in the article that this works for UPDATE queries, which was the original focus of this issue.

from IRC:

[5:20pm] davidstrauss: pwolanin, I would probably pick one of these strategies, in order (1) double-wrap the subquery, (2) extend the API to handle the special case of checking for existence of something in a foreign table, (3) give up and have Solr use a different query for mysql

So #2 would be an API addition. #1 is less efficient than the JOIN method, but easier to put in core. #3 is easiest, but leaves the rest of contrib and core out in the code.

Rough starting point.

The issue has absolutely nothing to do with updates. Subselects suck on SELECT or UPDATE equally. So while Damien's patch is right it just needs to verify the existence of a subselect and if it's not marked up for prevention then yes probably wrap it up.

Edit: In some cases, the optimizer code can execute the subquery for every row. That's not true. The truth is In every case MySQL runs the subquery for every row. MySQL only has dependent subqueries, no matter what. Anonymous views are, however, not translated into dependent subqueries only because that's simply not possible as the tables need to be resolved before the result set can even exist so independent subqueries are moved to an anonymous view.

@Crell - putting it into the condition object might be cleaner, since that's logically where it lives.

Revised patch fixing the comments and making this apply in all cases where we are running a subquery.

As already mentioned on IRC, I'm rather reluctant to wrap *all* subqueries into an additional SELECT. My reasoning is:

• For UPDATE and DELETE, this is actually a bug fix: MySQL by default doesn't support referencing the same table in the main query and the subquery (so, for example: DELETE FROM table WHERE id IN (SELECT id FROM table WHERE id > 0) will trigger an error). In that case the functionality is very broken, and it makes sense to fix it.
• For SELECT, this is just a performance optimization, and as a consequence is very much dependent on MySQL version.

For example, on MariaDB 5.1:

MariaDB [drupalorg]> pager cat > /dev/null
PAGER set to 'cat > /dev/null'

MariaDB [drupalorg]> DELETE FROM users WHERE uid IN (SELECT uid FROM users WHERE uid < 0);
ERROR 1093 (HY000): You can't specify target table 'users' for update in FROM clause

MariaDB [drupalorg]> SELECT uid FROM node;
601506 rows in set (0.41 sec)

MariaDB [drupalorg]> SELECT SQL_NO_CACHE * FROM users WHERE uid IN (SELECT uid FROM node);
86572 rows in set (4.07 sec)

MariaDB [drupalorg]> SELECT SQL_NO_CACHE * FROM users WHERE uid IN (SELECT * FROM (SELECT uid FROM node) _table);
Ctrl-C -- query killed. Continuing normally.
ERROR 1317 (70100): Query execution was interrupted

I had to interrupt the double-wrapper subquery, it ran for more then 20 minutes.

So this patch doesn't fix the bug?

How can we let a condition know that it's a condition on a update/delete versus a select?

The doxygen is totally useless. The rest is probably useful, yes.

More flexibility is good. Bot is happy, and we know that DBTNG is heavily tested.

#24: 961604-large-subselect-plumbing.patch queued for re-testing.

#24: 961604-large-subselect-plumbing.patch queued for re-testing.

Waiting on this for resolution to apachesolr issue #592522: Hooks node_type, taxonomy and user knocks out our database server

So it looks like the API change part of this would be internal to db_or() and db_and(), so existing modules should be unaffected. Yes?

However, I'm not seeing test coverage for:

a) The bug Crell found in #16.
b) Confirmation that the problem with crazy sub-queries like the one pwolanin wants to do in ApacheSolr is fixed.

I'm not sure if b) is possible, but it seems a) should be. Confirmation either way from pwolanin about this fixing the bug would be good.

I'm tempted to classify this as a feature request, but let's call it task in terms of "oversight" for now. Definitely not a bug.

Just as an update. Because of this missing feature we currently have to write our query in the module as the example below because mysql has a bug that prevents us from using the IN with a subquery.
Could you tell us how the patch above would fix this problem?

function apachesolr_user_update(&$edit, $account, $category) {
  if (isset($edit['name']) && $account->name != $edit['name']) {
    switch (db_driver()) {
      case 'mysql' :
        // UPDATE with inner join for Mysql only
        $query = "UPDATE {apachesolr_search_node} asn
          INNER JOIN {node} n ON asn.nid = n.nid SET asn.changed = :changed
          WHERE n.uid = :uid";
        $result = db_query($query, array(':changed' => REQUEST_TIME,
          ':uid' => $account->uid,
        ));
        break;
      default :
        // The IN condition with a subquery. Works properly in all databases except Mysql
        $nids = db_select('node')
          ->fields('node', array('nid'))
          ->where("uid = :uid", array(':uid' => $account->uid));
        $update = db_update('apachesolr_search_node')
          ->condition('nid', $nids, 'IN')
          ->fields(array('changed' => REQUEST_TIME))
          ->execute();
    }
  }
}

see #16:

3) Add a MySQL-specific DatabaseCondition class that preprocesses subqueries in where clauses into wrapped FROM subqueries, as per the article chx linked above.

Recategorizing.

Probably needs to wait to at least 8.1.

Last patch is from 6 years ago, and last real discussion from 5 years ago. Marking postponed since the IS needs updating if this is still relevant for Drupal 8.

Thank you for sharing your idea for improving Drupal.

We are working to decide if this proposal meets the Criteria for evaluating proposed changes. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or there is no community support. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Wanted to bump 1 more time if still needed?