Setting all search weights to 0 causes div by 0 error and stops indexing.

Gerhard Killesreiter - April 29, 2008 - 10:24
Project:Drupal
Version:5.x-dev
Component:node.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

Found by Robert

#1

flobruit - April 29, 2008 - 23:36
Status:active» needs review

To reproduce:

0. Enable the search module.
1. Set all search factor weights to 0 (this disables all of them).
2. Create a page node with the keyword "drupal".
3. Run cron.
4. Search for the keyword "drupal". Division by zero error in node.module (in the implementation of hook_search).

The node search code already defaults to using keyword relevance as the default search factor when none are specified:

$select2 = (count($ranking) ? implode(' + ', $ranking) : 'i.relevance') . ' AS score';

But count($ranking) is only zero when all search factors have a zero weight, meaning that their total weight is zero, which leads to the division by zero a little further in the code.

This patch doesn't change the algorithm, it just changes the code to use a proper if-statement. When using the default search factor, the total weight can safely be set to 1 (for those who are into mathematics and set theory, we can do this because 1 is the identity element of real numbers under multiplication, but that's beyond the scope of this patch).

AttachmentSizeStatusTest resultOperations
default_search_factor.patch1.94 KBIgnoredNoneNone

#2

Senpai - May 9, 2008 - 20:42
Status:needs review» reviewed & tested by the community

Tested and Working As Intended. I followed these steps:

1. Enabled the search module.
2. Set all search factor weights to 0.
3. Created a page node with the word "drupal" as the body text.
4. Created an article node with the word "drupal" as the body text.
5. Ran cron using devel.mod.
6. Searched for the keyword "drupal".
7. Received both pieces of content in the resulting list, with no divide-by-zero errors present in drupal_set_message().

#3

Dries - May 10, 2008 - 11:45
Version:7.x-dev» 6.x-dev
Status:reviewed & tested by the community» needs work

Committed to CVS HEAD. Thanks.

The patch did not apply against DRUPAL-6 so it will need a reroll.

#4

Dries - May 10, 2008 - 12:03

Committed to CVS HEAD. Thanks.

The patch did not apply against DRUPAL-6 so it will need a reroll.

#5

flobruit - May 12, 2008 - 16:50
Status:needs work» reviewed & tested by the community

Re-rolled for drupal 6. The only change was the spacing around the concatenation operator.

AttachmentSizeStatusTest resultOperations
default_search_factor.patch1.95 KBIgnoredNoneNone

#6

Gábor Hojtsy - May 19, 2008 - 08:07
Status:reviewed & tested by the community» fixed

Thanks, committed to Drupal 6!

#7

Anonymous (not verified) - June 2, 2008 - 08:35
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#8

catch - July 8, 2008 - 15:13
Version:6.x-dev» 5.x-dev
Status:closed» patch (to be ported)

Still a valid bug in D5.

#9

flobruit - July 10, 2008 - 10:43
Status:patch (to be ported)» reviewed & tested by the community

Patch still applied with some offset. Re-rolling without changes.

AttachmentSizeStatusTest resultOperations
default_search_factor-D5.patch2.01 KBIgnoredNoneNone

#10

drumm - July 16, 2008 - 19:04
Status:reviewed & tested by the community» fixed

Committed to 5.x.

#11

Anonymous (not verified) - July 31, 2008 - 04:46
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.