[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