Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hi,
I've applied the patch in #681760: Try to improve performance and eliminate duplicates caused by node_access table joins, and it works great at removing duplicate results in lists, however, it causes an SQL error in module grants when module_grants_OR_modules is set to FALSE (the default). The reason for this is that the above mentioned patch removes the join on the node access table, meaning that when module grants adds in its where clauses, the table alias 'na', does not exist, hence an error.
I managed to fix this by changing the default value of $node_access_alias that is passed too _module_grants_node_access_where_sql() to 'n' instead of 'na', and this seems to resolve the problem and module grants continues to work as expected.
I can't see any side affects from doing this, and just wanted to know if anyone could see any problems with making this change?
I'm not requesting a change be made to module grants, as I'm using a patched core, I'd rather like to know if anyone can see any problems with doing this.
I can't see any reason why there would be, as in effect 'na.nid' should always equal 'n.nid' anyway... Am I wrong?
Comments
Comment #1
RdeBoerI can't see any side-effects either.
To support your patch I have changed the method signature to:
function _module_grants_node_access_where_sql($node_op = 'view', $node_access_alias = 'n', $account = NULL)Checked into CVS (HEAD). Not in "official release" yet.
Comment #3
sethcohn CreditAttribution: sethcohn commentedI think that needs to be 'node' not 'n', as I was getting empty views and/or SQL errors with either the default of na (table didn't exist, as above) or n (the select was using node, not n, leading to another error).
I'm using patch in http://drupal.org/node/681760#comment-2658134
to fix the continuing drama (even in 6.16) of wrongly generated access calls.
Comment #4
cdale CreditAttribution: cdale commentedI am using the same patch you have posted too above, and 'n', works just fine. Perhaps there is another issue going on here? Or perhaps the query is being called from a different location than mine.
In any event, a more "robust" solution, would be to adjust module_grants_db_rewrite_sql() so that the call to _module_grants_node_access_where_sql() instead looks like the following:
Thoughts?
Comment #5
q0rban CreditAttribution: q0rban commentedSeems to be differing results here, so the maintainer will need some more info, steps to replicate, etc.
Comment #6
RdeBoerChecked into the Git repository master branch #1 and #4.