First, note that I reproduced this again HEAD, but I selected 4.7.0-beta2 as the Version, because 'cvs' appears many times in the version drop-down and I was afraid this would get lost if I selected the wrong 'cvs'.
The problem is the preg_replace on line 281 of database.inc can produce bad SQL. I inserted some code to print what the query looks like before and after the preg_replace, and here's what happens:
Query before preg_replace():
SELECT t.tid, t.*, parent FROM {term_data} t, {term_hierarchy} h WHERE t.tid = h.tid AND t.vid = %d ORDER BY weight, name
And after the preg_replace():
SELECT t.tid, t.*, parent FROM {term LEFT JOIN {term_data} catd ON t.tid = catd.tid _data} t, {term_hierarchy} h WHERE t.tid = h.tid AND t.vid = %d ORDER BY weight, name
As you can see, it inserted my join smack in the middle of {term_data}.
I made this small change to the preg_replace call, and it seems to fix the problem in my case. Not sure yet whether it breaks anything. Original code:
$query = preg_replace('|FROM[^[:upper:]/,_]+|','\0 '. $join .' ', $query);
My change eliminates ",_" from the regular expression:
$query = preg_replace('|FROM[^[:upper:]/]+|','\0 '. $join .' ', $query);
This puts my join after the list of tables and before the WHERE, which I think is where it belongs. Perhaps there is some reason for the ",_" in the regexp, but I don't see it.
It also seems to me the regexp will not work properly if "WHERE" were lower case.
Comment | File | Size | Author |
---|---|---|---|
#9 | db_rewrite_sql_3.patch | 624 bytes | chx |
Comments
Comment #1
chx CreditAttribution: chx commentedLet's delete underscore but do not delete the comma.
Comment #2
Dave Cohen CreditAttribution: Dave Cohen commentedIf you don't delete the comma, the join gets thrown in between "{term_data} t" and ", {term_hierarchy}". I think it belongs after both of those, and before the WHERE.
(Although mysql did not generate an error when the join was placed there.)
Comment #3
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedpossibly a dupe of http://drupal.org/node/38298
Comment #4
Steve Ratcliffe CreditAttribution: Steve Ratcliffe commentedThis bug was introduced recently in http://drupal.org/node/39257
Comment #5
chx CreditAttribution: chx commentedAssigning to myself, I am scheduling writing a new regexp. We will add the JOIN before the first JOIN. Stay tuned.
Comment #6
drewish CreditAttribution: drewish commentedchx, while you're in there could you update the function's comment to make it more descriptive? Something like: "Rewrites node queries to enforce access control. Only nodes to which the user has access permissions will be returned."
Comment #7
chx CreditAttribution: chx commentedNo. hooks docs live on contrib and not in code. Also it's also for taxonomy and now comments.
Comment #8
drewish CreditAttribution: drewish commentedI wasn't refering to the hook, but to the function itself. Currently it's simply:
Rewrites node queries.
That does a pretty crappy job of explaining what it's supposed to do. And as I'm sure you know the documentation is drawn from the comments in the code.
Comment #9
chx CreditAttribution: chx commentedTotally new regexp. Very simple, instead of putting it after the table name, we put the join expression before the first JOIN or whatever. Added some comments.
Comment #10
Cvbge CreditAttribution: Cvbge commentedWhat if there's no "JOIN or whatever" in the query?
Comment #11
chx CreditAttribution: chx commentedsee |$ at the end of preg: "or the end of string"
Comment #12
Dave Cohen CreditAttribution: Dave Cohen commented+1
Seems to work for me.
Comment #13
chx CreditAttribution: chx commentedSeems work to me means nothing. Which modules have you tried?
Comment #14
Dave Cohen CreditAttribution: Dave Cohen commentedWell, I mean that it fixes my problem which is the original reason I submitted this bug.
I'm using an access control module that I built myself. I'm happy to share it, but right now its not published anywhere. (I hoped to check it into drupal contrib, but because there is already taxonomy access control I was not granted a CVS account.)
Comment #15
chx CreditAttribution: chx commentedIf you are running a node access module then, I think, it's good to go.
Comment #16
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks.
Comment #17
Dries CreditAttribution: Dries commented