780820 - Allow multiple custom search criteria to match one field

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement)

Description

[:glob ✱]

The scope of a generic fix for bug 677757 is large, so I'm going to break it down a bit.

This bug is about adding the ability to group criteria to search against the same field for the change* operators only.  This mirrors the functionality which was in 4.0 and lost in 4.2.

Comment 1

[Max Kanat-Alexander]

FWIW, folks are going to miss it on Attachments too.

Comment 2

[Frédéric Buclin]

glob: ping?

Comment 3

[:glob ✱]

this is moving along, i'm finally seeing the light at the end of the tunnel.

i currently have a backend fix which works for all fields and operators except for the following fields:
  - classification
  - component
  - product
  - attachment data

i'm going to just mark those as 'not supported' for now.

now i have to work on the frontend template and javascript to hook this up.


how it works:  for each set of grouped rows, an inner join is created to the bugs table.  a variable has been added which contains the name of the bugs table, and a grouped clause class changes this variable for all its child conditions to point to the join table for that group.

Comment 4

[:glob ✱]

when i created this bug, it looked like it would be quicker to work on a limited set of fields.  as it panned out, this isn't the case.  i'll move things to bug 677757.

Comment 5

[:glob ✱]

as lpsolit's request, moving the work back to this bug.

Comment 6

[:glob ✱]

Attachment: patch v3

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 679195 [details] [diff] [review]
patch v3

Review of attachment 679195 [details] [diff] [review]:
-----------------------------------------------------------------

Missing ClauseGroup.pm from patch.

Comment 8

[:glob ✱]

Attachment: patch v4

pants, sorry about that.

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 680938 [details] [diff] [review]
patch v4

Review of attachment 680938 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 10

[David Lawrence [:dkl]]

Thanks Simon ;)

Comment 11

[Frédéric Buclin]

It passes all xt/ tests, right?

Comment 12

[:glob ✱]

(In reply to Frédéric Buclin from comment #11)
> It passes all xt/ tests, right?

xt/search.t .. ok           
All tests successful.
Files=1, Tests=65691, 38 wallclock secs ( 8.22 usr  0.42 sys + 18.64 cusr  3.26 csys = 30.54 CPU)
Result: PASS

i've filed bug 814320 to track adding tests for this new functionality to the xt testsuite.

Comment 13

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/DB.pm
modified Bugzilla/Search.pm
modified Bugzilla/DB/Oracle.pm
modified Bugzilla/Search/Clause.pm
added Bugzilla/Search/ClauseGroup.pm
modified js/custom-search.js
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/search/boolean-charts.html.tmpl
Committed revision 8482.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/DB.pm
modified Bugzilla/Search.pm
modified Bugzilla/DB/Oracle.pm
modified Bugzilla/Search/Clause.pm
added Bugzilla/Search/ClauseGroup.pm
modified js/custom-search.js
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/search/boolean-charts.html.tmpl
Committed revision 8466.

Comment 14

[Frédéric Buclin]

Comment on attachment 680938 [details] [diff] [review]
patch v4

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

> sub build_subselect {
>-    my ($outer, $inner, $table, $cond) = @_;
>-    return "$outer IN (SELECT $inner FROM $table WHERE $cond)";
>+    my ($outer, $inner, $table, $cond, $negate) = @_;
>+    # Execute subselects immediately to avoid dependent subqueries, which are
>+    # large performance hits on MySql
>+    my $q = "SELECT DISTINCT $inner FROM $table WHERE $cond";
>+    my $dbh = Bugzilla->dbh;
>+    my $list = $dbh->selectcol_arrayref($q);
>+    return $negate ? "1=1" : "1=2" unless @$list;
>+    return $dbh->sql_in($outer, $list, $negate);
> }

For the record, note that this is reverting what has been done in bug 579568 in Bugzilla 4.2, see https://bugzilla.mozilla.org/attachment.cgi?id=458134&action=diff#Bugzilla/Search.pm_sec3. Per bug 579568 comment 2, the original change was done for performance reasons, though no data was given to prove this.

Comment 15

[:glob ✱]

(In reply to Frédéric Buclin from comment #14)
> For the record, note that this is reverting what has been done in bug 579568
> in Bugzilla 4.2, see
> https://bugzilla.mozilla.org/attachment.cgi?id=458134&action=diff#Bugzilla/
> Search.pm_sec3. Per bug 579568 comment 2, the original change was done for
> performance reasons, though no data was given to prove this.

on mysql that's a massive performance regression, as that change makes them dependent subqueries.  what little benefit you gain regarding network latency, you lose by executing the subquery for each row.

Comment 16

[Frédéric Buclin]

(In reply to Byron Jones ‹:glob› (away until 2nd Jan) from comment #15)
> on mysql that's a massive performance regression, as that change makes them
> dependent subqueries.

Actually, the changes you made in build_subselect() severely regress queries involving comments, see bug 824262. A query which took < 0.1 second now takes ~12 seconds to complete.

Comment 17

[Frédéric Buclin]

Added to relnotes for 4.4rc2.

Comment 18

[balabarath]

how to apply this patch on my bugzilla? I already read https://wiki.mozilla.org/Bugzilla:Patches. But i can't figure it out.

Comment 19

[Andre Klapper]

balabarath: Basically download https://bug780820.bugzilla.mozilla.org/attachment.cgi?id=680938 and apply it via "patch < patchfile.diff" under Linux in the right directory. 
Going into detail how to apply patches is out of scope for this bug report. Feel free to ask on the Bugzilla support mailing list and provide more information about your Bugzilla version, Operating system, etc. 
Thanks for your understanding!