As I was thinking on the various usages of node_rewrite_sql , it came to my mind that with very little changes it can be generalized so that any queries against any table -- not just node -- can be rewritten. I have moved node_rewrite_sql to database.inc, renamed it db_rewrite_sql, introduced another necessary parameter, which does not affect the current (light) usage of node_rewrite_sql. I think the usage is even clearer now, 'cos I think nid_alias caused some confusion. Now it is called $primary_table , while the new parameter is called $primary_key.
What this would be good for? One may want to have permission and languages for taxonomy terms and vocabularies.
Comment | File | Size | Author |
---|---|---|---|
#22 | db_rewrite_bugfix.patch | 739 bytes | chx |
#20 | db_rewrite_fix_0.patch | 5.12 KB | chx |
#18 | db_rewrite_fix.patch | 5.11 KB | chx |
#17 | db_rewrite_empty_fix.patch | 2.88 KB | chx |
#15 | db_rewrite_sql_docfix.patch | 1.77 KB | chx |
Comments
Comment #1
Dries CreditAttribution: Dries commentedI guess it makes sense to go with a generic approach, though it is difficult to predict how many queries will end up being wrapped in a call to db_rewrite_sql() ... Comments?
Comment #2
Jose Reyero CreditAttribution: Jose Reyero commentedI like it!, though I still miss some more info about what the query is about -the 'hints' stuff- , but it's fine... :-)
But also think there should be some kind of 'guidelines' about wrapping specific queries or not. For the moment, my candidates are:
- node queries, which means 'queries that select lists of nodes', not any query which contains 'node'
- taxonomy queries, same here, 'queries that select lists of vocabularies/terms'
Both types of queries above are good candidates for joining permissions conditions, and maybe language conditions (i18n).
And maybe more in the future.... we'll see.
Comment #3
JonBob CreditAttribution: JonBob commentedComment #4
Jose Reyero CreditAttribution: Jose Reyero commentedComment #5
chx CreditAttribution: chx commentedA through test showed that introducing the $primary_key parameter was done incorrectly, thus the new version.
Comment #6
moshe weitzman CreditAttribution: moshe weitzman commentedI'm confusd about who is responsible for calling the rewrite function. Does it not make sense to run every query through this system (and thus insert a call into db_query). We already do this for prefixes. perhaps the hook is too performance draining?
Note that the rewrite sql() hook is not documented on the hooks page: http://drupaldocs.org/api/head/group/hooks. One would document it by editing http://cvs.drupal.org/viewcvs/contrib/contributions/docs/developer/hooks/
Comment #7
chx CreditAttribution: chx commentedThere have been talks on #drupal about automatizing, and even the simpler node_rewrite_query was found to be better called by hand. But this? Automatic mechanism for this -- I think -- is almost impossible.
Comment #8
chx CreditAttribution: chx commentedI feel some cold coming... yes, it's the code freeze approaching. Please help this patch into core :)
Comment #9
chx CreditAttribution: chx commentedHere is the patch with search-and-replace done.
Comment #10
chx CreditAttribution: chx commentedAnd here is a version against current CVS.
Comment #11
Dries CreditAttribution: Dries commentedCommitted to HEAD.
Comment #12
chx CreditAttribution: chx commentedDocumentation fix (and the doc is a bit nicer, too). There is no code change.
Comment #13
Gábor HojtsySet to patch then...
Comment #14
Dries CreditAttribution: Dries commentedBecause of a bug in the project module, you can't use '.inc' in the name of your patch. Please rename and resubmit your patch.
Comment #15
chx CreditAttribution: chx commentedSo here it is.
Comment #16
Dries CreditAttribution: Dries commentedCommitted to HEAD.
Comment #17
chx CreditAttribution: chx commentedWhile writing term_node and vocabulary rewrites for taxonomy module (patch is under testing, will post this evening), I've unearthed a nasty bug with this.
If $where is empty -- which did not happened with n type rewrites, 'cos node_access_where_sql always returns at least '1' -- then the resulting SQL will be invalid, there will be an empty AND clause.
While I was at it, I made the whole thing faster -- replacements are only done when they are needed.
Comment #18
chx CreditAttribution: chx commentedAs Jose pointed out, node_db_rewrite_sql is wrong, it shall check for primary_field not for primary_table. Also, primary_field is better name than primary_key.
Previous patch is also included in this one.
Comment #19
Dries CreditAttribution: Dries commentedThis patch does not apply against HEAD:
2 out of 3 hunks FAILED -- saving rejects to file includes/database.inc.rej
.Comment #20
chx CreditAttribution: chx commented:(
Comment #21
Dries CreditAttribution: Dries commentedCommitted to HEAD.
Comment #22
chx CreditAttribution: chx commentedSteef stumbled on another bug.
Comment #23
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedIndeed, after applying the patch the node_* access modules works again without spitting errors and other things...
Please apply this patch to HEAD...
Comment #24
matt westgate CreditAttribution: matt westgate commentedIndeed +1
It's difficult to do any node-level permissions work when Drupal's throwing 'Fatal errors' because of syntax errors in database.inc.
Comment #25
chx CreditAttribution: chx commentedTo my best knowledge, the biggest hurdle this patch faces is the sad fact that the power supply of Dries' loved developer notebook died... Have patience, please.
Comment #26
Steven CreditAttribution: Steven commentedApplied to CVS.
Comment #27
(not verified) CreditAttribution: commented