Wrong SELECT syntax breaks db_rewrite_sql
dkruglyak - March 13, 2007 - 17:59
| Project: | Help Tip |
| Version: | 4.7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Dave Cohen |
| Status: | closed |
Description
I found my apache log flooded with warnings. On closer inspection looks like db_rewrite_sql inserts DISTINCT where it does not belong (inside LEFT JOIN statement). Here is rewritten SQL out of the logfile:
SELECT ht.*
FROM node n
LEFT JOIN helptip ht ON ht.DISTINCT(n.nid) = n.nid
LEFT JOIN helptip_user_data ht_hidden ON ht_hidden.uid=0 AND ht_hidden.nid=n.nid AND ht_hidden.relation='hidden'
INNER JOIN node_access na ON na.nid = n.nid
WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR
(na.gid = 0 AND na.realm = 'og_public') OR
(na.gid = 0 AND na.realm = 'og_all'))) AND
n.status = 1 AND
ht_hidden.relation IS NULL AND
('node/28189' LIKE ht.path OR 'feed/items/system/2007/03/13/borat_from_the_cutting_room_floor' LIKE ht.path) AND
weight >= -10 AND weight <= 10
ORDER BY ht.weight, RAND() LIMIT 1 The problem is regular expression in db_distinct_field, which does not understand the SQL fed to it by helptip. The fix is trivial tweak of one line where SELECT statement is:
Original code (excerpt):
$result = db_query(db_rewrite_sql("SELECT ht.* FROM {node} n ...Replacement code:
$result = db_query(db_rewrite_sql("SELECT nid FROM {node} n ...Attached module file can be commited to 4.7. This still needs to be propagated into 5.0.
| Attachment | Size |
|---|---|
| helptip.module.txt | 13.67 KB |
#1
I'm travelling at the moment. Will commit this when I get a chance, but not sure when that will be.
Thanks for the detailed report and fix. In the future please try to submit patches in the form of a diff, rather than a complete module file.
#2
yogadex, there is really not much to review here.
It is a trivial change of select column syntax (4 characters) which is not material but is needed to pass db rewrite muster.
I will try to learn how to create patch files properly...
#3
Could we commit the fix to 4.7 and 5.1? Trivial and ready to go...
#4
Here's the actual patch I've checked in (to DRUPAL-4-7, DRUPAL-5 and HEAD). It's slightly different from your original, so please open this bug again if your log warnings persist.
Thanks for the reminder, I was out of the country for a couple months. I suspect this is not the only thing I dropped the ball on.
#5
Forgot the change status.
#6