Search index process broken on mysql >= 5.0.13

Posted by gotheric on October 19, 2006 at 2:03pm

5 followers

Issue Summary

With Mysql >= 5.0.13 search index process does not work correctly.

The reason is a change in GREATEST mysql function. As stated on official Mysql site:
"Before MySQL 5.0.13, GREATEST() returns NULL only if all arguments are NULL. As of 5.0.13, it returns NULL if any argument is NULL."
(from http://dev.mysql.com/doc/refman/5.0/en/comparison-operators.html)

During the search indexing process the node_update_index() function in node.module launch this query:
$result = db_query_range('SELECT GREATEST(c.last_comment_timestamp, n.changed) as last_change, n.nid FROM {node} n LEFT JOIN {node_comment_statistics} c ON n.nid = c.nid WHERE n.status = 1 AND ((GREATEST(n.changed, c.last_comment_timestamp) = %d AND n.nid > %d) OR (n.changed > %d OR c.last_comment_timestamp > %d)) ORDER BY GREATEST(n.changed, c.last_comment_timestamp) ASC, n.nid ASC', $last, $last_nid, $last, $last, $last, 0, $limit);

If c.last_comment_timestamp is NULL the function GREATEST(c.last_comment_timestamp, n.changed) should return n.changed. But with the new GRATEST behavious it returns NULL.

The result is that the node will be skipped (and if more than $limit nodes are in that situation NO NODES WILL BE INDEXED at all).

The query should be changed in this way:
$result = db_query_range('SELECT GREATEST(if(c.last_comment_timestamp is NULL, 0, c.last_comment_timestamp), n.changed) as last_change, n.nid FROM {node} n LEFT JOIN {node_comment_statistics} c ON n.nid = c.nid WHERE n.status = 1 AND ((GREATEST(n.changed, if(c.last_comment_timestamp is NULL, 0, c.last_comment_timestamp)) = %d AND n.nid > %d) OR (n.changed > %d OR c.last_comment_timestamp > %d)) ORDER BY GREATEST(n.changed, if(c.last_comment_timestamp is NULL, 0, c.last_comment_timestamp)) ASC, n.nid ASC', $last, $last_nid, $last, $last, $last, 0, $limit);

Comments

#1

Posted by chx on October 20, 2006 at 12:04am

Version:

4.7.4

» x.y.z

#2

Posted by chx on October 20, 2006 at 12:04am

Status:

needs review

» active

I wanted to change this... mmmm

#3

Posted by chx on October 21, 2006 at 3:56pm

Status:

active

» needs review

Attachment	Size	Status	Test result	Operations
greatest_5013.patch	1.45 KB	Ignored: Check issue status.	None	None

#4

Posted by profix898 on October 21, 2006 at 6:36pm

I just tried to review this patch using MySQL 5.0.24 (PHP 5.1.6 on WinXPsp2). But actually indexing works fine even without this patch! Are there any special settings/modules/... required to reproduce this bug?

#5

Posted by chx on October 22, 2006 at 6:09am

I think you need comment module disabled and a node added in this state. That node won't be indexed.

#6

Posted by profix898 on October 22, 2006 at 7:38am

Status:

needs review

» needs work

Thanks, I didnt realize that comment.module is enabled by default ;)

To review I added 95 random nodes, then disabled the comment.module and added another 5 with known (searchable) content. On the search settings page AFTER the cron run the 'Indexing status' was 95%, what (I think) means the 95 random nodes were indexed but the other 5 were not. But actually performing a search (for a word in the known nodes) I got results even from the 5 (not indexed?) nodes.
With this patch the index status AFTER cron is 100% and all content seems indexed.

BUT the patch doesnt work with PostgreSQL!

Warning: pg_query() [function.pg-query]: Query failed: ERROR: function if(boolean, integer, integer) does not exist
in C:\Programme\xampp\htdocs\drupalcvs\includes\database.pgsql.inc on line 120

Warning: ERROR: function if(boolean, integer, integer) does not exist
query: SELECT GREATEST(IF(c.last_comment_timestamp IS NULL, 0, c.last_comment_timestamp), n.changed) as last_change, n.nid FROM node n LEFT JOIN node_comment_statistics c ON n.nid = c.nid WHERE n.status = 1 AND ((GREATEST(n.changed, c.last_comment_timestamp) = 0 AND n.nid > 0) OR (n.changed > 0 OR c.last_comment_timestamp > 0)) ORDER BY GREATEST(n.changed, c.last_comment_timestamp) ASC, n.nid ASC LIMIT 20 OFFSET 0 in C:\Programme\xampp\htdocs\drupalcvs\includes\database.pgsql.inc on line 139

I'm not a PostgreSQL/SQL expert, so I dont know there is a solution that works with every db. Maybe we need a switch() {} as in hook_install to get a db-specific solution!?

#7

Posted by chx on October 22, 2006 at 9:02am

Status:

needs work

» needs review

Your pgsql install is wrong, there was a time when a bug in system.install caused the functions not install.. Try a fresh setup.

<?php
      db_query('CREATE OR REPLACE FUNCTION "if"(boolean, integer, integer) RETURNS integer AS
        \'SELECT CASE WHEN $1 THEN $2 ELSE $3 END;\'
        LANGUAGE \'sql\''
      );
?>

#8

Posted by profix898 on October 22, 2006 at 9:26am

Status:

needs review

» reviewed & tested by the community

@chx: You are right, I tried with latest code but using an existing db. The FUNCTION "if" stuff is only added in hook_install, but not in a hook_update_x, what means it is not available after an upgrade to Drupal 5, right?

However the patch works nicely with a clean install on both MySQL and PostgreSQL. RTBC?

#9

Posted by sammys on October 22, 2006 at 3:26pm

the if function was already added to a 4.7 database in database.pgsql and will still be available after an upgrade.

#10

Posted by Dries on October 22, 2006 at 5:06pm

Another solution might be to make last_comment_timestamp default to '0' instead of NULL. Not sure if that has any implications, but from a code's point of view, it might be a tad more elegant.

With that in mind, I decided to look at the database definition and it reads like this:

modules/system/system.install:      last_comment_timestamp int NOT NULL default '0',
modules/system/system.install:      last_comment_timestamp integer NOT NULL default '0',

Wouldn't that mean that GREATEST() can never return NULL? Both changed and last_comment_timestamp should be non-NULL ...