I wrote a module called cac_lite which provides simple access control for users of the category module. One of the things it does is rewrite sql queries where the $primary_field is 'cid' (category id). Now I've learned of another module (Comment RSS) that also calls db_rewrite_sql with $primary_field 'cid'. But in this case 'cid' stands for comment id. The effect is that cac_lite rewrites a sql query that it shouldn't rewrite. So users can't have commentrss and cac_lite installed together. The underlying problem is that implementers of hook_db_rewrite_sql have no accurate test for what the fully-qualified primary field is. They are passed a $primary_table, but this is usually an alias of the table's real name. For instance, when commentrss calls it, $primary_table is 'c' and not 'comments'. When category calls it, $primary_table is again 'c' and not 'category'. What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not? -Dave
While I personally think that hook_db_rewrite_sql is The Devil's Work, there are a couple of ways around this feature of the hook. First, if you can, make use of the "primary table" parameter. The second is easier, or harder, depending upon how much code you have in your module. The best advice is to use a more descriptive variable name than $cid, assuming it's your variable, and not out of one of the taxonomy modules. The best would be if in 4.8, the d**n hook was redesigned to make it easier to be clear when you want your hook implementation to get called, and when you don't want to get called. There is currently no easy way to avoid this kind of problem. With the increasing use of the Views module, we're going to get more and more conflicts creeping in via db_rewrite_sql problems like this. Rob Dave Cohen wrote:
I wrote a module called cac_lite which provides simple access control for users of the category module. One of the things it does is rewrite sql queries where the $primary_field is 'cid' (category id).
Now I've learned of another module (Comment RSS) that also calls db_rewrite_sql with $primary_field 'cid'. But in this case 'cid' stands for comment id.
The effect is that cac_lite rewrites a sql query that it shouldn't rewrite. So users can't have commentrss and cac_lite installed together.
The underlying problem is that implementers of hook_db_rewrite_sql have no accurate test for what the fully-qualified primary field is. They are passed a $primary_table, but this is usually an alias of the table's real name. For instance, when commentrss calls it, $primary_table is 'c' and not 'comments'. When category calls it, $primary_table is again 'c' and not 'category'.
What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not?
-Dave
What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not?
You could, awkwardly, test the full $query for your table's name and alias, e.g. if (strstr($query, '{category} c')) { }
What about that last param in db_rewrite_sql() for extra args? I've been working on a massive Tagadelic patch (ugh...) and have passed in array('omit_check' => 'tagadelic') for certain db_rewrite_sql calls that I don't want passing through taxonomy visibility settings. It's probably not the most desirable solution, but has worked for the time being. I don't think this is what that last param in db_rewrite_sql() is designed for this, but I could be wrong. Rob Roy Barreca Founder and COO Electronic Insight Corporation http://www.electronicinsight.com rob@electronicinsight.com Nedjo Rogers wrote:
What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not?
You could, awkwardly, test the full $query for your table's name and alias, e.g.
if (strstr($query, '{category} c')) {
}
Nedjo, I was hoping it would not come to that. But it looks like the only solution that does not require changes in other people's modules. Thanks, -Dave On Tuesday 15 August 2006 15:33, Nedjo Rogers wrote:
What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not?
You could, awkwardly, test the full $query for your table's name and alias, e.g.
if (strstr($query, '{category} c')) {
}
Hi Dave, I think that you brought the 'cid' issue to my attention before. I am willing to let 'cid' be changed to 'cat_id' or 'category_id', in order to fix this problem. As far as I'm concerned, if a particular field name is already 'taken' by a core module, then a contrib module (such as category) should respect that. Having said that, I believe there are examples of field name conflicts even within core. For example, 'fid' is used both as 'file ID' (by the 'files' table), and as 'field ID' (by the 'profile_fields' table). IMHO, the most sensible route to take is to accept that field name conflicts are inevitable, and to make sure that db_rewrite_sql is able to handle them (and to improve db_rewrite_sql if it can't handle them). Cheers, Jaza. On 8/16/06, Dave Cohen <drupal@dave-cohen.com> wrote:
Nedjo,
I was hoping it would not come to that. But it looks like the only solution that does not require changes in other people's modules. Thanks,
-Dave
On Tuesday 15 August 2006 15:33, Nedjo Rogers wrote:
What's a module to do when it recieves two calls to hook_db_rewrite_sql; in both cases $primary_field is 'cid' and $primary_table is 'c', but in one case the query needs re-writing and in the other it does not?
You could, awkwardly, test the full $query for your table's name and alias, e.g.
if (strstr($query, '{category} c')) {
}
I think we should strive very strongly to prevent key name conflicts. Even if it means giving them better names... It breeds much less confusion when you have many tables.... On Wed, 2006-08-16 at 12:04 +1000, Jeremy Epstein wrote:
Hi Dave,
I think that you brought the 'cid' issue to my attention before. I am willing to let 'cid' be changed to 'cat_id' or 'category_id', in order to fix this problem. As far as I'm concerned, if a particular field name is already 'taken' by a core module, then a contrib module (such as category) should respect that.
Having said that, I believe there are examples of field name conflicts even within core. For example, 'fid' is used both as 'file ID' (by the 'files' table), and as 'field ID' (by the 'profile_fields' table). IMHO, the most sensible route to take is to accept that field name conflicts are inevitable, and to make sure that db_rewrite_sql is able to handle them (and to improve db_rewrite_sql if it can't handle them).
Cheers, Jaza.
On 8/16/06, Dave Cohen <drupal@dave-cohen.com> wrote:
Nedjo,
I was hoping it would not come to that. But it looks like the only solution that does not require changes in other people's modules. Thanks,
-Dave
participants (6)
-
Darrel O'Pry -
Dave Cohen -
Jeremy Epstein -
Nedjo Rogers -
Rob Barreca -
Rob Thorne