The current code for drupal 5.18 (encountered the bug on) and 6.12 adds a ranking clause like so by default:

%d * POW(2, (GREATEST(n.created, n.changed, c.last_comment_timestamp) - %d) * 6.43e-8)

This is then used to calculate the final relevance of all nodes that were brought up in the search results. However, if there is no entry in node_comment_statistics for a given node, c.last_comment_timestamp will = NULL. In both the versions of mysql I tested with GREATEST(any number, NULL) will return NULL; subtraction with NULL returns NULL; multiplication with NULL returns NULL. Finally, the relevance is returned as NULL.

The attached patch is against drupal 5. Drupal 6 has MAX(c.last_comment_timestamp), and MAX(NULL) is also NULL. So, the patch to drupal 6 could simply be MAX(c.last_comment_timestamp) || 1 (or some such low number). Something simily might need to be applied in the 'status' switch in node_search in drupal 5, but I'll leave that up to someone who knows the code better.

Files: 
CommentFileSizeAuthor
#19 488166-fix-reroll.patch1.26 KBjhodgdon
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-reroll.patch.
[ View ]
#18 488166-fix-search-relevance-d6.19.patch1.07 KBEmanueleQuinto
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-search-relevance-d6.19.patch.
[ View ]
#7 488166-fix-search-relevance-d6.16.patch992 bytesEmanueleQuinto
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-search-relevance-d6.16.patch.
[ View ]
#5 488166-fix-search-relevance-d6.patch1.6 KBbrianV
#2 search_relevance_fix_drupal6.txt729 bytesjhutton
node_search_fix.txt734 bytesjhutton

Comments

Version:5.18» 6.x-dev
Status:Needs review» Needs work

+1 subscribe.

Should be fixed in 5.x and 6.x. This is not an issue in D7 as last_comment_timestamp isn't used.

Version:6.x-dev» 6.12
Status:Needs work» Needs review
StatusFileSize
new729 bytes

Bumped to 6.12 since the bug also appears to be present there. It looks as if Drupal 7 does not contain the bug, however, since it does
'score' => 'POW(2.0, (GREATEST(n.created, n.changed) - %d) * 6.43e-8)'
leaving out last_comment_timestamp entirely.

Attached is a patch against drupal 6.12.

Version:6.12» 6.x-dev
Status:Needs review» Reviewed & tested by the community

Looks good.

Bumping back to 6,x-dev since that is where it would be fixed.

Status:Reviewed & tested by the community» Needs work

I'd like to request a line of comment above this code. I see that since 1 will be lower then the other MAX-es, you get some real value, but that would be good to document. Otherwise it could easily get people puzzled on what the hell happens there.

Status:Needs work» Needs review
StatusFileSize
new1.6 KB

Updated, with a comment explaining things. The comment is sort of long, but as short as I could make it while adequately explaining the issue.

Problem is still there in 6.16

patch should now be:

$ranking[] = '%d * POW(2, (GREATEST(MAX(n.created), MAX(n.changed), MAX(c.last_comment_timestamp) || 1) - %d) * 6.43e-8)';

StatusFileSize
new992 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-search-relevance-d6.16.patch.
[ View ]

Here attached patch for 6.16

.. and still there in 6.17 too

... the patch works for 6.17 and 6.x-dev too (as for node.module,v 1.947.2.24 2010/06/02 12:11:07 goba), it only seems to need review.

Status:Needs review» Reviewed & tested by the community

Patch applies, search now works properly - thanks!

Marking as tested.

Status:Reviewed & tested by the community» Fixed

Great, thanks for the testing. Committed.

Priority:Normal» Critical
Status:Fixed» Active

Sorry to reopen, but this is very broken:

  • Dealing with NULL values is precisely what the COALESCE() function is for
  • || is not a portable operator for OR, it's the ANSI operator for string concatenation
  • || 1 doesn't really make any sense :p

The search module is now broken on PostgreSQL. Yay, fun.

What you wanted here is: GREATEST(MAX(n.created), MAX(n.changed), COALESCE(MAX(c.last_comment_timestamp), 0)).

It would also probably makes more sense to move the MAX() out of the GREATEST(), but the result would be unaffected and it's another issue anyway.

Damien meant to say that #882194: PostgreSQL query error when searching was marked as a duplicate, I think.

Thanks Jennifer, that somewhat makes more sense.

Title:Search relavence calculation fails if last_comment_timestamp is NULLSearch relevance calculation fails if last_comment_timestamp is NULL

It seems solved by removing the || 1 part. A 'coalesce' function is not needed because the timestamp is NOT NULL; just the greatest value of 3 timestamps is needed.

GREATEST(MAX(n.created), MAX(n.changed), MAX(c.last_comment_timestamp))

@jpulles: have you seen the title of the issue? :)

StatusFileSize
new1.07 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-search-relevance-d6.19.patch.
[ View ]

This patch should be at least SQL-92 conformant. It uses COALESCE now.

I don't have a PostgreSQL environment, apologize for this, then should be tested there...

Status:Active» Reviewed & tested by the community
StatusFileSize
new1.26 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 488166-fix-reroll.patch.
[ View ]

Just as a note: When supplying patches for Drupal, it's best to (if possible) roll them using cvs diff, and at a minimum, roll them from the Drupal root, rather than at the level of the particular file you are patching.

That said, I have set up a PostgreSQL environment, reproduced the SQL error that is happening now, then applied the patch above. The errors went away, and my searching tests (on contrib module Porter Stemmer) now pass.

So here is the same patch, rerolled from Drupal head.

@Damien: Yes, I read the title, but thought that the last_comment_timestamp could never be null since it is defined as 'NOT NULL' in the database schema.

Many people are reporting this -- WE NEED TO FIX IT SOON!!!!
#888164: After update search has ceased to work
http://drupal.org/node/885026
etc.

Subscribe.

Anyone having this problem using Postgres and Drupal 6.19 can apply the patch in #19 and likely fix the problem (if someone besides me could verify, that might help).

If you are not familiar with how to apply patches, take a look at the attached patch file. Basically the lines starting with - need to be removed and replaced with the lines starting with +. You can do this by hand in a text editor, since this patch is so small. Be sure to remove the + sign at the beginning of the two lines you are adding!!

Yes. I had same issue here using 6.19, Postgres. Fixed manually. Thanks.

I'm not familiar with the testing process here... and am confused by:

  • article status says, "reviewed & tested by the community", but
  • attachment status is "Test request sent", while Test Result is "None".

I don't know how to contribute my two cents worth, which is that the patch fixed my problem on 6.19 with PG 8.3.7.

#19 Works for me. (Debian Lenny apache2+postgres)

subscribe

#19 works for me postgres 8.4.4

Works for me on PostgreSQL 8.4.4! Woohoo! Thank you jhodgdon for making this patch!

OMG! patching now...

Patch worked for me. THANK YOU!

For those who don't want to patch their production site I found the following worked as a temporary measure.
Go to Admin>Settings>Search and set the Factor 'Recently Posted' to 0. (Will have to remember to set back again on the next release).

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 488166-fix-reroll.patch, failed testing.

Thanks christoph for this note, as no one has solved this problem yet and some of us with postgres don't want to be in the wild with core security vulnerabilities.

Status:Needs work» Reviewed & tested by the community

D6 patches are failing with the same error. There's no problem with this patch.

Status:Needs work» Reviewed & tested by the community

I wasn't aware of that, thank you.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 488166-fix-reroll.patch, failed testing.

re-testing is not going to help.
#961172: All D6 Core patches are failing

This is my error:

user warning: Operand should contain 1 column(s) query: SELECT COUNT(*) FROM (SELECT i.type, i.sid, 5 * (3,6765835972501 * SUM(i.score * t.count)) + 5 * POW(2, (GREATEST(MAX(n.created), MAX(n.changed), COALESCE(MAX(c.last_comment_timestamp), 0)) - 1288716585) * 6.43e-8) + 5 * (2.0 - 2.0 / (1.0 + MAX(c.comment_count) * 0,14285714285714)) + 5 * (2.0 - 2.0 / (1.0 + MAX(nc.totalcount) * 0,0050251256281407)) AS score FROM search_index i INNER JOIN search_total t ON i.word = t.word INNER JOIN node n ON n.nid = i.sid INNER JOIN active_translation ON n.nid = active_translation.es LEFT JOIN node_comment_statistics c ON c.nid = i.sid LEFT JOIN node_counter nc ON nc.nid = i.sid WHERE n.status = 1 AND (i.word = 'parroquia') AND i.type = 'node' GROUP BY i.type, i.sid HAVING COUNT(*) >= 1) n1 in ../search/search.module on line 958.

My partner show that in sql change the "," for "." work i find the solution...

diaxpro: is this the same problem as has been reported above, and does the patch in #19 above fix it?

If it is a separate problem, please file a new issue. Thanks.

I'm sorry but #19 don't work for me and the solution that i find is in http://drupal.org/node/389452 on coment #2
Thx.

Your problem is different from this error report. Your problem is that SQL doesn't support commas as decimal points, but for some reason, your system is creating queries with commas as decimal points. The solution you found is telling PHP not to do that. So it's unrelated to this issue.

Patch #19 works on Ubuntu 8, PG 8.3.12 and D 6.19.

I'm very surprised. Many users report that the patch #19 works.

Why is not taken over the patch?

Thanks @#32! brilliant thought!

#19: 488166-fix-reroll.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 488166-fix-reroll.patch, failed testing.

Status:Needs work» Reviewed & tested by the community

I had thought that #961172: All D6 Core patches are failing was fixed, and so I asked for a retest of this patch, but it's not fixed.

Hi,

Please release a new version with this patch (#45) as it totally breaks things in postgresql here. I did exactly this fix (coalesce) and it works here.

Is it possible to generate another release at least with this correction ? This is a critical bug and it is fixed, so there is not holding it.

Thanks for the work.

#32 Thank you! A simple and elegant solution. I only have to remember to modify it later ^_^

#19 & #32 worked for me.

Drupal 6.19
Postgres 8.4

Thanks! ;)

Status:Reviewed & tested by the community» Fixed

Thanks, committed.

Status:Fixed» Closed (fixed)

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

thanks my friend for that configuration , i was looking for the solution of this problem a long time ago and this setting works perfectly , thanks so much