[development] Forum module bug challenge

Earl Miles merlin at logrus.com
Tue Jul 24 07:21:26 UTC 2007


There's a bug in forum.module that I wanted to fix this cycle, and I realize I 
am not going to get to it. I've got entirely too many things on my plate (Views 
2, I'm looking at you) and I need to divest myself of some of these things I 
want to do. However, this one is important.

template_preprocess_forum_topic_navigation (formerly 
theme_forum_topic_navigation) has this *really* awful query in it:

function template_preprocess_forum_topic_navigation(&$variables) {
   $output = '';

   // get previous and next topic
   $sql = "SELECT n.nid, n.title, n.sticky, l.comment_count, 
l.last_comment_timestamp FROM {node} n INNER JOIN {node_comment_statistics} l 
ON n.nid = l.nid INNER JOIN {term_node} r ON n.nid = r.nid AND r.tid = %d WHERE 
n.status = 1 ORDER BY n.sticky DESC, ". 
_forum_get_topic_order_sql(variable_get('forum_order', 1));
   $result = db_query(db_rewrite_sql($sql), isset($variables['node']->tid) ? 
$variables['node']->tid : 0);

   $stop = $variables['prev'] = $variables['next'] = 0;

   while ($topic = db_fetch_object($result)) {
     if ($stop == 1) {
       $variables['next'] = $topic->nid;
       $variables['next_title'] = check_plain($topic->title);
       $variables['next_url'] = url("node/$topic->nid");
       break;
     }
     if ($topic->nid == $variables['node']->nid) {
       $stop = 1;
     }
     else {
       $variables['prev'] = $topic->nid;
       $variables['prev_title'] = check_plain($topic->title);
       $variables['prev_url'] = url("node/$topic->nid");
     }
   }
}

To summarize: It runs a query to find every post in the current forum, then 
counts its way through to your current post, then looks 1 ahead to see what the 
next/previous posts are. If you have thousands of topics in your forum, this is 
extremely inefficient.

This can be much more efficiently done in two queries where you do a limit 1 
and do > on one and < on the other, using the sort criteria, to determine what 
next/previous is.

For example, if you're sorting on created, you do a SELECT nid FROM node WHERE 
[sufficient clauses to restrict to just this forum] WHERE node.created >= 
$node->created AND node.nid <> $node->nid LIMIT 1

And reverse it to find the previous.

Here's the challenge: Write the patch to fix this, cause this needs to happen. 
I checked the issue queue and I don't see an issue for it.


More information about the development mailing list