Support subqueries, db_rewrite_sql broken

one obvious solution is to not send anything through db_rewrite_sql that should be rewritten. the caller can inject his subquery afterwards. if the subquery has to be rewritten, he can send that through separately.

an alternate solution is for the caller to add a hint in the db_rewrite_sql() call and replace some token with the suquery with example_rewrite_sql(). in other words, implement the hook yourself and inject there.

there may be better options though.

This is a patch.

which no longer applies.

To also clarify, I do not think that this is not a problem with the new search implementation...

Er, could you clarify - it's not a problem, or it is a problem?

subqueries are lovely and it'd be great to get this in, but I'm not sure how to test this..

Thanks for the write-up. I can confirm that testing with all core modules enabled, adding, deleting users, nodes, taxonomy terms etc. is fine. Complex queries like search and tracker are unaffected. Wish I could help with the seasoned developer bit!

i am a bit bold tonight and calling this critical. IMO, we upped system requirements in part so we can use subqueries in Drupal6. we can't have a key function like db_rewrite_sql() actively discourage their use. once can't avoid that function - it enforces our whole node access control. every Views query uses it as well.

The WHERE you need is not the first nor the last.
SELECT nid FROM (SELECT nid FROM term_node WHERE tid = 1) AS x INNER JOIN (SELECT nid FROM term_node WHERE tid = 1) AS y USING (nid) INNER JOIN foo USING (nid) WHERE (a = B) OR (weight IN (SELECT weight FROM allowed_weights WHERE (allow = 1))) AND q = 1 and I guess I could write even uglier pieces. The WHERE you need is the one outside of balanced parentheses, in other words ^(?P<anonymous_view>(?:[^()]++|\((?P>anonymous_view)\))*).*?WHERE. However, we want the last of GROUP|ORDER|LIMIT and there is no point in putting HAVING in there because that requires the presence of GROUP.

As the anonymous_view subpattern had a * quantifier having a ? after it is not needed.

OK well I won't pretend to know what's going on in #16, but it doesn't break anything I tested either (adding/deleting nodes, users, taxonomy terms, searching, tracker etc.)

I think the patch removes support for "HAVING".

But that's fine as said in my original post, GROUP BY comes before HAVING. Anyways, the patch above found the first group/order/limit instead of the last. We would need strrpos for strings. That's PHP5. We can emulate.

No breakage with #19 either.

No need to retest, this is a trivial cleanup: $needle is not used so I removed it.

Try this and still note that your test case does not use a subquery after the main while. Note this version is also a cleanup: uses one less preg.

adding/deleting nodes, adding/deleting user, adding/deleting term all fine. No time for more comprehensive testing until later, possibly tomorrow - so wouldn't hurt for an additional review from someone else.

No change at all, just added back that comment. SELECT nid FROM node WHERE nid IN (SELECT nid FROM somewhere WHERE foo = 'bar') this kind of subquery will make your version fail IMO.

Thanks for the patch and for all the testing. Hopefully we can make this a bit more nicer come Drupal 7 on PHP 5.

http://drupal.org/node/222128

To elaborate: This patch has broken the use of HAVING without GROUP BY which is IMO perfectly valid SQL. http://www.contrib.andrew.cmu.edu/~shadow/sql/sql1992.txt does not mention that GROUP BY would be required of HAVING is used:

If neither a <where clause> nor a <group by clause> is speci-
fied, then let T be the result of the preceding <from clause>;
if a <where clause> is specified, but a <group by clause> is
not specified, then let T be the result of the preceding <where
clause>; otherwise, let T be the result of the preceding <group
by clause>.

moving

Moving back to D6, chx explained that db_rewrite_sql will be removed in D7.

Hi everyone,
Here is a patch in order to enable HAVING clause without GROUP BY clause. It's what #31 report.

I was bit by this today and #26 solved my issue. Drupal5 postgresql 8.3. Thanks chx and doug!

Yes, definitely #26++! I used it on 5.7 (without problems) to solve a huge MySQL error I was getting. I'm not sure what can go into point releases, but would it be possible to also release this as a bug fix for 5 when it finally gets resolved? I'm writing code for some 5.x contrib modules that will depend on this patch.

(Should I have opened a new bug for this?)
db_rewrite_sql in Drupal 6.3 seems to be similar to #26. With it I get different results between PHP 4.3.11 and PHP 5.2.3. I've nailed the problem down to the actual preg_match-call. So I think there is some change in how PHP works between these versions, which affects queries where we do not want to use the last WHERE of the string.

 //Test case. The $pattern is equivalent to that in #26 but shorter to write.
$q = "SELECT (SELECT * FROM {x} WHERE 1) AS inner1
FROM {node} node_SHOULD_END_HERE WHERE
node.vid IN (SELECT tn.vid FROM {term_node} tn_ERROR_MATCHED_TOO_MUCH WHERE tid = %d)";
$pattern = '/^((?P<anonymous_view>(?:[^()]++|\((?P>anonymous_view)\))*)[^()]+WHERE)/';
preg_match($pattern, $q, $matches);
print "<pre>\n{$matches[1]}\n</pre>";

On PHP 5.2.3 I get the expected:

SELECT (SELECT * FROM {x} WHERE 1) AS inner1
 FROM {node} node_SHOULD_END_HERE WHERE

While on PHP 4.3.11 the output is:

SELECT (SELECT * FROM {x} WHERE 1) AS inner1
 FROM {node} node_SHOULD_END_HERE WHERE
 node.vid IN (SELECT tn.vid FROM {term_node} tn_ERROR_MATCHED_TOO_MUCH WHERE

The erroneous PHP4 output means that db_rewrite_sql will put additional WHERE-code (like node access restrictions) in the inner query ({term_node} in my case). In certain views this can cause a SQL error.

Any ideas about how to overcome this difference, so that PHP4 is equally supported? On PHP5 #26 (and probably #34) works fine.

I now have a solution to #37 so that db_rewrite_sql works the same way on PHP 4.3.11 as on PHP 5.2.3. This version is probably a bit slower, 60-90% when only measuring the affected lines, but I doubt it would be worth using different code for PHP4 and PHP5.
The attached patch applies to the Drupal 6.3 release. If desired, #34 can still be applied on top of this.

I posted something related here (http://drupal.org/node/162970#comment-1009000) before notice that issue is a duplicate for this one.

subscribing

Obviously this isn't a major issue in D6 as a year without activity indicates. Maybe this issue should be won't fixed as community focus is shifting to D7 and code freeze and D6 seems to be humming along fine without this patch.

Thanks very much for the patches here, I applied emok's (#38) and it fixed a real headache I was having with SQL syntax errors which seem to have been down to how the original pattern worked. I'm afraid I haven't been able to figure out why it has fixed it (I'm new to PHP and Drupal) but my post explains the issue in case anyone's interested: http://drupal.org/node/585458.