Project: Drupal Version: cvs Component: database system Category: bug reports Priority: normal Assigned to: chx Reported by: chx Updated by: stefan nagtegaal Status: patch Indeed, after applying the patch the node_* access modules works again without spitting errors and other things... Please apply this patch to HEAD... stefan nagtegaal Previous comments: ------------------------------------------------------------------------ January 24, 2005 - 19:58 : chx Attachment: http://drupal.org/files/issues/db_rewrite_sql.patch (7.66 KB) 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. ------------------------------------------------------------------------ January 24, 2005 - 23:08 : Dries I 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? ------------------------------------------------------------------------ January 24, 2005 - 23:28 : Jose A Reyero I 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. ------------------------------------------------------------------------ January 24, 2005 - 23:41 : chx Attachment: http://drupal.org/files/issues/db_rewrite_sql_0.patch (7.68 KB) A through test showed that introducing the $primary_key parameter was done incorrectly, thus the new version. ------------------------------------------------------------------------ January 24, 2005 - 23:43 : moshe weitzman I'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/ ------------------------------------------------------------------------ January 25, 2005 - 00:00 : chx There 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. ------------------------------------------------------------------------ January 25, 2005 - 20:54 : chx I feel some cold coming... yes, it's the code freeze approaching. Please help this patch into core :) ------------------------------------------------------------------------ January 27, 2005 - 20:34 : chx Attachment: http://drupal.org/files/issues/db_rewrite_sql_1.patch (37.83 KB) Here is the patch with search-and-replace done. ------------------------------------------------------------------------ January 29, 2005 - 18:49 : chx Attachment: http://drupal.org/files/issues/db_rewrite_sql_2.patch (38.67 KB) And here is a version against current CVS. ------------------------------------------------------------------------ January 29, 2005 - 23:03 : Dries Committed to HEAD. ------------------------------------------------------------------------ January 30, 2005 - 10:41 : chx Attachment: http://drupal.org/files/issues/database.inc.docfix.patch (1.77 KB) Documentation fix (and the doc is a bit nicer, too). There is no code change. ------------------------------------------------------------------------ January 30, 2005 - 14:20 : Goba Set to patch then... ------------------------------------------------------------------------ January 30, 2005 - 17:51 : Dries Because of a bug in the project module, you can't use '.inc' in the name of your patch. Please rename and resubmit your patch. ------------------------------------------------------------------------ January 30, 2005 - 17:57 : chx Attachment: http://drupal.org/files/issues/db_rewrite_sql_docfix.patch (1.77 KB) So here it is. ------------------------------------------------------------------------ January 30, 2005 - 18:49 : Dries Committed to HEAD. ------------------------------------------------------------------------ January 30, 2005 - 19:17 : chx Attachment: http://drupal.org/files/issues/db_rewrite_empty_fix.patch (2.88 KB) While 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. ------------------------------------------------------------------------ January 30, 2005 - 19:58 : chx Attachment: http://drupal.org/files/issues/db_rewrite_fix.patch (5.11 KB) As 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. ------------------------------------------------------------------------ January 31, 2005 - 20:29 : Dries This patch does not apply against HEAD: 2 out of 3 hunks FAILED -- saving rejects to file includes/database.inc.rej. ------------------------------------------------------------------------ January 31, 2005 - 20:40 : chx Attachment: http://drupal.org/files/issues/db_rewrite_fix_0.patch (5.12 KB) :( ------------------------------------------------------------------------ February 1, 2005 - 19:46 : Dries Committed to HEAD. ------------------------------------------------------------------------ February 3, 2005 - 16:01 : chx Attachment: http://drupal.org/files/issues/db_rewrite_bugfix.patch (739 bytes) Steef stumbled on another bug. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111