Replace db_rewrite_sql() with hook_query_alter().

Here they are:
http://api.drupal.org/api/function/db_rewrite_sql/7/references

subscribe

New plan, in the correct order:

1) Implement node_query_alter() (a DISABLED_node_query_alter() is in, but is marked as "don't work yet")
2) Convert all db_rewrite_sql() that rewrite node queries using the new query builder, and tag them with 'node_access'
3) Remove node_db_rewrite_sql() (yeah!!)
4) Think of a plan for other rewritten queries (taxonomy, ...)
5) Execute the plan defined in 4
6) Remove db_rewrite_sql() (re-yeah!!)

7) Profit! ;)

I greatly look forward to committing this patch when it's ready. :D

Subscribing.

I was just thinking about this. i will review and give some feedback.

In order to test this, it is best IMO to upgrade http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/exam... to use the new API.

I committed an update for that node_access_example module so it can be used in HEAD to generate sensible node access records ... I hope to look at this patch soon.

Is there an example out there of of a query that used to do pager_query() but now does dynamic select? If not, could you give some pointers or even just paste in a converted query from node_page_default() ... I am assuming that we must do dynamic in order to add a tag.

Also, I would like to remove the generic node_query_alter hook and instead go to the specific ones that form_alter does like hook_form_alter_[form_id]. we would do hook_query_alter_[tag].

Yes paging and node access are independant but node access queries are always node listings by definition. Each one that I can find is either a pager_query or a db_query_range(). I don't know how to add a tag to those.

With Crell's help, I have worked though the bugs in the previous patch and converted one forum block to use the new node access system. It is going to be a biggish job to convert all the other node listing queries to be dynamic queries so I'd prefer to have that happen in a follow up patch. Also, there are some patches in the queue that will make those queries simpler (e.g. addFields method).

The forum block does not use node_title_list() since I could not see how to use it now that the query builder can alias our fields, willy nilly.

There is a small bug fix to the DB API - added missing parens.

Actually, we can use node_title_list() as is. Simpler patch attached.

This patch has no dependencies so feel free to commit it once it is RTBC :)

In order to easily convert the rest of the node listing queries, we ought to get these in:

• #320591: Tag specific alter hook
• #299267: Add "Extender" support to SELECT query builder
• #318440: Make it easier to add multiple fields in a dynamic SELECT

subscribe

Reroll

I think we are good to go, the patch is rather simple. Yay db_rewrite_sql dies.

While this patch is relatively easy indeed, I'd still like to get a better handle on how other queries would get converted. Therefore, I'm going to investigate #320591: Tag specific alter hook and #299267: Add "Extender" support to SELECT query builder some more first to see if that helps me wrap my head around that question.

Committed to CVS HEAD. Thanks.

Those handbook docs need no further updates IMO. The upgrade docs need to mention this but we really need to commit #320591: Tag specific alter hook and #299267: Add "Extender" support to SELECT query builder to do a good job explaining the conversion process.

hook_query_alter() needs an example in the docs.

Reposting for bot's sake.

Committed to CVS HEAD. Thanks.

Don't know if you noticed but there's a typo in this patch

$td_alias = $query->join('taxonomy_term_data', 'td', '***tn.tid*** = tn.tid');

Seems like this should be td.tid.

@RandalK The original patch that has the typo was committed quite a bit ago. Can you check core and make sure the typo still exists and open up a new issue maybe referencing this issue?

It is still present in forum_block_view():

        $query = db_select('node', 'n');
        $tn_alias = $query->join('taxonomy_term_node', 'tn', 'tn.vid = n.vid');
        $td_alias = $query->join('taxonomy_term_data', 'td', 'tn.tid = tn.tid');
        $l_alias = $query->join('node_comment_statistics', 'l', 'n.nid = l.nid');

I'm assuming that there are no tests for that block, or it would have been caught. Probably not as part of this patch, but adding a test would be nice :)

Committed to CVS HEAD. I'm marking this 'code needs work' because we don't have a test yet for the forum block.

Created a separate issue in forum.module for the followup and now we can mark this as fixed.
#440344: Tests needed for forum blocks

It's in the API & here:
http://drupal.org/node/224333#db_rewrite_sql

Are more docs needed?