Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: Dries Status: patch 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? Dries Previous comments: ------------------------------------------------------------------------ January 24, 2005 - 20: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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch 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. chx 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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch 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. chx 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/ -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/db_rewrite_sql_1.patch (37.83 KB) Here is the patch with search-and-replace done. chx 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 :) -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: Goba -Status: fixed +Status: patch Set to patch then... Goba 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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/db_rewrite_sql_docfix.patch (1.77 KB) So here it is. chx 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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch 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. chx 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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/db_rewrite_fix_0.patch (5.12 KB) :( chx 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. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111

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

Project: Drupal Version: cvs Component: database system Category: bug reports Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch To 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. chx 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. ------------------------------------------------------------------------ February 3, 2005 - 16:10 : stefan nagtegaal Indeed, after applying the patch the node_* access modules works again without spitting errors and other things... Please apply this patch to HEAD... ------------------------------------------------------------------------ February 4, 2005 - 23:46 : mathias Indeed +1 It's difficult to do any node-level permissions work when Drupal's throwing 'Fatal errors' because of syntax errors in database.inc. -- View: http://drupal.org/node/16111 Edit: http://drupal.org/project/comments/add/16111