I wanted to reformat a little the forum navigation elements, so I started looking at the API for theme_forum_topic_navigation. I was rather perplexed by the contents of this function. For example, it calls a (nominally) private forum.module function _forum_get_topic_order_sql in order to get its basic work done.

This use of a "private" function plus the complex SQL query seems wholey incompatible with my notion of a themeable function. Also, I would imagine that doing this query plus running through the logic of this function must be a bit costly for a site like drupal.org with lots of forums.

So, here's where I would think about going with this: Add columns to the "forum" table for previous/next nid (and title?). Rather than generating the previous/next for every page load, regenerate it at a fixed interval using a cron hook. Then the themeable function could be passed the current $node with the title & nid of the previous/next topics.

Am I way off base here? Is there any (real) advange to running this query every time I look at a forum page?

Comments

pwolanin’s picture

pwolanin commented

-----
>Date: Fri, 28 Jul 2006 10:49:07 -0500 (CDT)
>From: "Larry Garfield"
>Subject: Re: [development] Database queries on drupal.org

>- Someone needs to get that query out of the forum module's theme >code. :-)

follow ups here to the above comment on the development list

pwolanin’s picture

pwolanin commented

Thinking more about this. Here are the four possible orderings for forums;

Date - newest first
Date - oldest first
Posts - most active first
Posts - least active first

The news/oldest terminology here is misleading, since the order is determined by last comment time, not
node creation time. Maybe a separate patch?

For the first two, the order only needs to be updated when a forum topic is added (or updated?). For the latter two, the order needs to be revised when a forum topic is added or when a new comment is added.

So, forum_insert (and forum_update for changed titles) and a new forum_comment (hook_comment) functions need to deal with this.

Does this approach make sense- seems like updating the forum table under these conditions might be more of a performance hit than the existing query?

Would a better alternative be to separate the query out into a helper function and do caching?

pwolanin’s picture

pwolanin commented
Status: Active » Closed (duplicate)

now duplicate to this: http://drupal.org/node/72617