365179 - Performance problems with multiple fields chosen in the "where one or more of the following changed" box

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Dave Miller [:justdave]]

Our SQL when multiple fields are selected in the "where one or more of the following changed:" box is bad, and is causing severe performance problems on b.m.o.

Comment 1

[Dave Miller [:justdave]]

Attachment: EXPLAIN on that query

Comment 2

[Frédéric Buclin]

joel, any clue? The problem comes from Search.pm around line 354:

push(@supptables, "LEFT JOIN bugs_activity AS actcheck " .
                  "ON (" . join(" OR ", @actlist) . "$extra )");

Comment 3

[Dave Miller [:justdave]]

We should use IN() on the list of fieldids instead of a big block of ORs.  MySQL does a binary search on the contents of an IN() which is a lot faster than comparing a bunch of ORs.

Also, the way this is written currently makes me thing that only the last field in the list is actually going to work, because I think AND has precedence over OR, and there's an AND adjacent to the last fieldid.

Comment 4

[Frédéric Buclin]

Attachment: untested patch, v0.1

I haven't tested this patch *at all* and I have no idea if it's correct or not, nor whether it fixes the perf issue. So it really needs to be tested.

Comment 5

[Dave Miller [:justdave]]

Comment on attachment 249809 [details] [diff] [review]
untested patch, v0.1

This works, and works great.  Still accurate, and cuts the time for running that specific query down to about 2 seconds (from over 40 minutes, that's a hell of an improvement).

>+                my $extra = " ON actcheck.bug_id = bugs.bug_id";

I'd prefer to cut the ON out of here ^^^

>                 push(@supptables, "LEFT JOIN bugs_activity AS actcheck " .
>-                                  "ON (" . join(" OR ", @actlist) . "$extra )");
>+                                  "$extra AND actcheck.fieldid IN (" . join(",", @actlist) . ")");

and put it here ^^^ instead.  Makes it a little more readable.

    push(@supptables, "LEFT JOIN bugs_activity AS actcheck " .
                      "ON $extra AND actcheck.fieldid IN (" . join(",", @actlist) . ")");

Carry forward r+ with that change.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

Carrying forward justdave's r+

Comment 7

[Frédéric Buclin]

Is 2.22 affected too?

Comment 8

[Frédéric Buclin]

(In reply to comment #7)
> Is 2.22 affected too?
> 

Hum, looking at bug 276232, it seems that 2.22 is not affected as it doesn't have this OR thing.

Comment 9

[Dave Miller [:justdave]]

Comment on attachment 249813 [details] [diff] [review]
patch, v1

>+                my $extra = " actcheck.bug_id = bugs.bug_id";

nit: you don't need the space ^^^ at the beginning of the string here

>+                                  "ON $extra AND actcheck.fieldid IN (" .

because you have it here --------------^

Comment 10

[Frédéric Buclin]

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.144; previous revision: 1.143
done