Last Comment Bug 578494 - QuickSearch for phrases is busted
: QuickSearch for phrases is busted
Status: RESOLVED FIXED
: regression
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 3.6.1
: All All
-- major (vote)
: Bugzilla 3.6
Assigned To: Max Kanat-Alexander
: default-qa
:
Mentors:
https://bugzilla.mozilla.org/buglist....
: 578576 (view as bug list)
Depends on:
Blocks: bmo-regressions-1007 553884
  Show dependency treegraph
 
Reported: 2010-07-13 14:54 PDT by Jesse Ruderman
Modified: 2010-08-06 19:51 PDT (History)
12 users (show)
mkanat: approval+
mkanat: approval4.0+
mkanat: blocking4.0+
mkanat: approval3.6+
mkanat: blocking3.6.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (3.6 branch) (809 bytes, patch)
2010-07-13 21:16 PDT, Reed Loden [:reed] (use needinfo?)
mkanat: review-
Details | Diff | Splinter Review
v1 (1.96 KB, patch)
2010-07-19 21:04 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review
Pg: No shellwords, v1 (1.65 KB, patch)
2010-08-01 16:58 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2010-07-13 14:54:05 PDT
If I try to QuickSearch for a phrase, I get 200 random bugs that don't even contain the second word.  This almost prevents me from getting work done.
Comment 1 User image Max Kanat-Alexander 2010-07-13 20:11:02 PDT
reed: I'm not entirely sure that this is an upstream bug. Can you reproduce it on landfill? I'm seeing a lot of bugs like "my search returns random results" reported against bmo.
Comment 2 User image Reed Loden [:reed] (use needinfo?) 2010-07-13 21:16:00 PDT
Created attachment 457245 [details] [diff] [review]
patch - v1 (3.6 branch)

Here's a patch that should work for the 3.6 branch. 4.0 and trunk need a better fix.
Comment 3 User image Max Kanat-Alexander 2010-07-13 21:17:14 PDT
Comment on attachment 457245 [details] [diff] [review]
patch - v1 (3.6 branch)

This won't work on Pg, as mentioned on IRC:

 reed: The upstream problem is that we don't actually have a fulltext engine for Pg.
 reed: So we have to understand quotes inside of sql_fulltext or whatever it's called, in Bugzilla/DB.pm.
 reed: That could probably be accomplished by using splitwords from Text::ParseWords instead of just splitting the string.
Comment 4 User image Max Kanat-Alexander 2010-07-13 21:18:20 PDT
  Or shellwords, more specifically--that tends to work the most simply.
Comment 5 User image Reed Loden [:reed] (use needinfo?) 2010-07-13 21:26:29 PDT
Comment on attachment 457245 [details] [diff] [review]
patch - v1 (3.6 branch)

Landed on bmo 3.6 branch as a temp hack until a better fix is made.

Committing to: bzr+ssh://bzr.mozilla.org/bmo/3.6/
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7160.
Comment 6 User image Frédéric Buclin 2010-07-14 04:45:12 PDT
Looks like a dupe of bug 556579.
Comment 7 User image Max Kanat-Alexander 2010-07-14 09:05:30 PDT
(In reply to comment #6)
> Looks like a dupe of bug 556579.

  No, that bug only affects 4.0, and this bug is against 3.6.
Comment 8 User image Max Kanat-Alexander 2010-07-19 20:53:44 PDT
Before I go about this: I'm assuming that Oracle fulltext understands quotes mean "this is a phrase", yes?
Comment 9 User image Max Kanat-Alexander 2010-07-19 21:04:03 PDT
Created attachment 458553 [details] [diff] [review]
v1

This should fix it on 3.6, 4.0, and trunk, I'm pretty sure.
Comment 10 User image Max Kanat-Alexander 2010-07-19 21:41:50 PDT
*** Bug 578576 has been marked as a duplicate of this bug. ***
Comment 11 User image Reed Loden [:reed] (use needinfo?) 2010-07-22 14:04:26 PDT
Comment on attachment 458553 [details] [diff] [review]
v1

No time to get to this until minimum of next week.
Comment 12 User image Frédéric Buclin 2010-07-22 16:34:25 PDT
Comment on attachment 458553 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Search/Quicksearch.pm'

>+sub _matches_phrase {
>+    $phrase =~ s/"/\\"/g;

Seems to work, but I'm not sure about to trigger this line as shellwords() splits phrases on quotes. r=LpSolit
Comment 13 User image Frédéric Buclin 2010-07-22 16:34:55 PDT
I didn't check if it applies on 4.0 and 3.6.
Comment 14 User image Max Kanat-Alexander 2010-07-22 18:47:56 PDT
Thanks! It should apply on 4.0 and 3.6 also--there haven't been any significant changes to that code since 3.6.
Comment 15 User image Max Kanat-Alexander 2010-07-22 18:51:38 PDT
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/DB.pm
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7397.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/DB.pm
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7341.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/DB.pm
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7144.
Comment 16 User image Max Kanat-Alexander 2010-07-22 19:12:31 PDT
Something about this fix is breaking the fulltext search, according to the xt/search tests on our tinderboxen.
Comment 17 User image Max Kanat-Alexander 2010-08-01 16:58:55 PDT
Created attachment 461961 [details] [diff] [review]
Pg: No shellwords, v1

Okay, this fixes the problem with Pg. The problem was that shellwords absolutely can't handle unbalanced quotes--it returns an empty result if you do something like: "don't try this" for content/matches (without the double-quotes). So instead, I'm doing a hack just to allow Quicksearch to work properly on Pg.
Comment 18 User image Max Kanat-Alexander 2010-08-01 16:59:54 PDT
BTW, there's also currently some taint error on Pg for quicksearch, on HEAD. I have to find out what it is.
Comment 19 User image Frédéric Buclin 2010-08-01 18:13:48 PDT
Comment on attachment 461961 [details] [diff] [review]
Pg: No shellwords, v1

Seems to work fine. Let's hope we don't regress anything else. :) r=LpSolit
Comment 20 User image Max Kanat-Alexander 2010-08-01 18:17:48 PDT
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified Bugzilla/DB.pm
Committed revision 7415.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/                         
modified Bugzilla/DB.pm
Committed revision 7358.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/                         
modified Bugzilla/DB.pm
Committed revision 7151.
Comment 21 User image Daniel Veditz [:dveditz] 2010-08-05 16:26:11 PDT
This is "fixed" (upstream), do we need another bug to get this patch deployed so it's actually fixed for BMO users?
Comment 22 User image Max Kanat-Alexander 2010-08-05 16:59:28 PDT
(In reply to comment #21)
> This is "fixed" (upstream), do we need another bug to get this patch deployed
> so it's actually fixed for BMO users?

  Well, no, I think Mozilla just needs to employ some person who actually has time to work on bugzilla.mozilla.org. (Currently all maintenance is being done by volunteers, really.)
Comment 23 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2010-08-06 19:51:01 PDT
If it got committed to the 3.6 branch upstream, then we should have it on b.m.o already.  I just merged it again yesterday when 3.6.2 released.

Note You need to log in before you can comment on or make changes to this bug.