179697 - OR terms need extra level of bracketing

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Bradley Baetz (:bbaetz)]

The url here does

'short_desc' 'allwordssubstr' 'foo bar' OR
'status_whiteboard' 'allwordssubstr' 'foo bar'

This becomes

(INSTR(LOWER(bugs.short_desc), 'foo') AND INSTR(LOWER(bugs.short_desc), 'bar') OR
INSTR(LOWER(bugs.status_whiteboard), 'foo') AND
INSTR(LOWER(bugs.status_whiteboard), 'bar'))

instead, it should be:

((INSTR(LOWER(bugs.short_desc), 'foo') AND INSTR(LOWER(bugs.short_desc), 'bar')) OR
(INSTR(LOWER(bugs.status_whiteboard), 'foo') AND
INSTR(LOWER(bugs.status_whiteboard), 'bar')))

Note the extra bracketing. Without this, the query is read left to right, and is
asking for all bugs where:

short_desc contains 'foo' AND (short_desc contains 'bar' OR (whiteboard contains
foo AND whiteboard contains bar)))

which isn't optimisiable easily, and so this then becomes basically a full db
search. The bugzilla sidebar generates queries like this - see bug 179500.

The fix is to change

            if (@orlist) {
                push(@andlist, "(" . join(" OR ", @orlist) . ")");
            }

in Bugzilla::Search to do the @orlist join by bracketing each term first before
doing the bracketing for pusshin to @andlist.

To make the queries simpler/easier to read, we may want to only add the extra
bracketing if scalar(@orlist) > 1.

Comment 1

[Bradley Baetz (:bbaetz)]

Attachment: fix

I don't think that this should break any valid query (famous last words, I
know...)

Comment 2

[Bradley Baetz (:bbaetz)]

-> me

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: explains on the queries before and after the patch

The patch certainly *seems* right, but it doesn't have any measurable impact. 
Here are explains from the query in the URL as well as from a "foo bar" search
with the Mozilla search sidebar.  In both cases the explains are the same
before and after the patch, and the queries take exactly the same time to run.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 106027 [details] [diff] [review]
fix

this seems sensible nonetheless. r=myk

Comment 5

[Dave Miller [:justdave]]

a= justdave

Comment 6

[Bradley Baetz (:bbaetz)]

Fixed.

The lack of a difference between queries is because mysql doesn't optimise OR stuff.