[development] Forum module bug challenge

[catch libcom]

Earl Miles wrote:

template_preprocess_forum
>
> _topic_navigation (formerly
> theme_forum_topic_navigation) has this *really* awful query in it:


[..]


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.

_________


I think this is the issue (although nothing in there for that particular
query).

http://drupal.org/node/145353 (forum performance improvements)

I have an abortive patch on there (the forum table disappeared as I was
working on it) which was a start on:

   - forum_get_topics
   - forum_get_forums
   - forum_topics_unread

Also: http://drupal.org/node/148849 (merge {node_comment_statistics} and
{node_counter} into {node}

which'd affect that query.

catch


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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.drupal.org/pipermail/development/attachments/20070724/50771706/attachment.htm