Looking at this function which raps db_query calls and allows modules to alter the query, I was wondering how a hook_db_rewrite_sql is supposed to distinguish the queries that it wants to alter from the queries that it wants to leave alone? Only way I can think of is to do a string match on large parts of the original query, but isn't this quite messy, or am I missing something? Could someone please shed the light on this issue?
Looking at this function which raps db_query calls and allows modules to alter the query, I was wondering how a hook_db_rewrite_sql is supposed to distinguish the queries that it wants to alter from the queries that it wants to leave alone? This is something that still needs some work IMO. Right now you can access the primary table (i.e. 'n' for node, but I always hated how it didn't use the full pre-prefixed table name) and field (i.e. 'nid'). This is pretty scary considering we now have such clean implementations for other drupal_alter()isms.
I know I have participated in a discussion like this before, but couldn't find the issue. So here it is again: What about passing a unique identifier along with every call of db_rewrite_sql? Instead of just wrapping db_rewrite_sql() we could also pass $module and $key so we have accurate namespaces for each query. (Or, we could just do it like hook_mail_alter(), but I think there could be namespace conflicts if a module named foo_module's foo_module_key screws with a module named foo's foo_module_key :P) I know we have $args, but that is already being used for actual args IIRC. What about replacing db_rewrite_sql('SELECT * FROM ...', 'node_query_select_foo', 'n', 'nid', $args) with some form of drupal_alter() call function db_rewrite_sql(...) { $query = 'SELECT * FROM ...'; drupal_alter('query', $query, 'node_query_select_foo', 'node', 'nid', $args) return $query; } I feel like this is much cleaner so you know exactly which query you are rewriting as opposed to just analyzing the query, primary table/field. If we made this really clean and precise maybe we could open to door to writing even better access modules. But then again...code is gold. Rob Roy Barreca Founder and COO Electronic Insight Corporation http://www.electronicinsight.com rob@electronicinsight.com Ashraf Amayreh wrote:
Looking at this function which raps db_query calls and allows modules to alter the query, I was wondering how a hook_db_rewrite_sql is supposed to distinguish the queries that it wants to alter from the queries that it wants to leave alone?
Only way I can think of is to do a string match on large parts of the original query, but isn't this quite messy, or am I missing something? Could someone please shed the light on this issue?
I know I have participated in a discussion like this before, but couldn't find the issue. So here it is again: What about passing a unique identifier along with every call of db_rewrite_sql? Instead of just wrapping db_rewrite_sql() we could also pass $module and $key so we have accurate namespaces for each query. (Or, we could just do it like hook_mail_alter(), but I think there could be namespace conflicts if a module named foo_module's foo_module_key screws with a module named foo's foo_module_key :P)
this was proposed in the original db_rewrite_sql() but didn't get in. We called them query hints or somesuch ... We had enough to chew on at that time. It is time to try again, as you propose.
+1 on rethinking this. I was unable to use db_rewrite_sql for a views add-on because what I really needed was some information about the view being modified. I'm not sure if (a) db_rewrite_sql is broken or (b) it could just be used better. Having just reviewed the uses of db_rewrite_sql, it seems that the final argument $args to db_rewrite_sql is never used (at least by core). I can only presume that this argument was meant to provide module specific context. But no modules are using it. If this was used more often, then a hook_db_rewrite_sql implementation would have some context. I don't have a proposal, but would like to see this discussion for 6.x. I couldn't find the original proposal and discussion. Can anyone point me to it? Thanks! Doug Green 904-583-3342 www.douggreenconsulting.com Bringing Ideas to Life with Software Artistry and Invention... Providing open source software political solutions -----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Moshe Weitzman Sent: Tuesday, April 17, 2007 9:39 PM To: development@drupal.org Subject: Re: [development] db_rewrite_sql
I know I have participated in a discussion like this before, but couldn't find the issue. So here it is again: What about passing a unique identifier along with every call of db_rewrite_sql? Instead of just wrapping db_rewrite_sql() we could also pass $module and $key so we have accurate namespaces for each query. (Or, we could just do it like hook_mail_alter(), but I think there could be namespace conflicts if a module named foo_module's foo_module_key screws with a module named foo's foo_module_key :P)
this was proposed in the original db_rewrite_sql() but didn't get in. We called them query hints or somesuch ... We had enough to chew on at that time. It is time to try again, as you propose.
Moshe Weitzman wrote:
I know I have participated in a discussion like this before, but couldn't find the issue. So here it is again: What about passing a unique identifier along with every call of db_rewrite_sql? Instead of just wrapping db_rewrite_sql() we could also pass $module and $key so we have accurate namespaces for each query. (Or, we could just do it like hook_mail_alter(), but I think there could be namespace conflicts if a module named foo_module's foo_module_key screws with a module named foo's foo_module_key :P) The thing is that it's not about uniquely identifying queries, but about rewriting some kind of queries whatever module they come from. I.e. some modules would want to rewrite *all* node listing queries. So what we really need there is just some more information about 'function' and some 'context' but not really an identifier like in the case of $form_id. I'm talking abt passing parameters like 'node_listing', 'vocabulary_loading' or 'administration', 'content', 'view', etc...
this was proposed in the original db_rewrite_sql() but didn't get in. We called them query hints or somesuch ... We had enough to chew on at that time. It is time to try again, as you propose.
Yes, It's about time we improve it a little bit. That function got the job done at that time and I think so far it's proven to be quite useful for modules - og, i18n :-) But really we could add some more information there so we don't have to relay anymore on preg_match or path for guessing about the kind of query it is.
participants (5)
-
Ashraf Amayreh -
Doug Green -
Jose A. Reyero -
Moshe Weitzman -
Rob Barreca