Bug 780820: Allows for multiple custom search criteria to match one field

r=dkl,a=LpSolit

5 files changed, 155 insertions, 32 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 7756ff53b..877e6cb15 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -385,8 +385,10 @@ sub sql_string_until {
 }
 
 sub sql_in {
-    my ($self, $column_name, $in_list_ref) = @_;
-    return " $column_name IN (" . join(',', @$in_list_ref) . ") ";
+    my ($self, $column_name, $in_list_ref, $negate) = @_;
+    return " $column_name "
+             . ($negate ? "NOT " : "")
+             . "IN (" . join(',', @$in_list_ref) . ") ";
 }
 
 sub sql_fulltext_search {
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index fcc4e88a9..01c37a028 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -203,16 +203,16 @@ sub sql_position {
 }
 
 sub sql_in {
-    my ($self, $column_name, $in_list_ref) = @_;
+    my ($self, $column_name, $in_list_ref, $negate) = @_;
     my @in_list = @$in_list_ref;
-    return $self->SUPER::sql_in($column_name, $in_list_ref) if $#in_list < 1000;
+    return $self->SUPER::sql_in($column_name, $in_list_ref, $negate) if $#in_list < 1000;
     my @in_str;
     while (@in_list) {
         my $length = $#in_list + 1;
         my $splice = $length > 1000 ? 1000 : $length;
         my @sub_in_list = splice(@in_list, 0, $splice);
         push(@in_str, 
-             $self->SUPER::sql_in($column_name, \@sub_in_list)); 
+             $self->SUPER::sql_in($column_name, \@sub_in_list, $negate));
     }
     return "( " . join(" OR ", @in_str) . " )";
 }
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 8746c2706..3d09def65 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -23,6 +23,7 @@ use Bugzilla::Group;
 use Bugzilla::User;
 use Bugzilla::Field;
 use Bugzilla::Search::Clause;
+use Bugzilla::Search::ClauseGroup;
 use Bugzilla::Search::Condition qw(condition);
 use Bugzilla::Status;
 use Bugzilla::Keyword;
@@ -1151,8 +1152,8 @@ sub _translate_join {
     
     die "join with no table: " . Dumper($join_info) if !$join_info->{table};
     die "join with no 'as': " . Dumper($join_info) if !$join_info->{as};
-        
-    my $from_table = "bugs";
+
+    my $from_table = $join_info->{bugs_table} || "bugs";
     my $from  = $join_info->{from} || "bug_id";
     if ($from =~ /^(\w+)\.(\w+)$/) {
         ($from_table, $from) = ($1, $2);
@@ -1557,7 +1558,7 @@ sub _charts_to_conditions {
     my $clause = $self->_charts;
     my @joins;
     $clause->walk_conditions(sub {
-        my ($condition) = @_;
+        my ($clause, $condition) = @_;
         return if !$condition->translated;
         push(@joins, @{ $condition->translated->{joins} });
     });
@@ -1577,7 +1578,7 @@ sub _params_to_data_structure {
     my ($self) = @_;
     
     # First we get the "special" charts, representing all the normal
-    # field son the search page. This may modify _params, so it needs to
+    # fields on the search page. This may modify _params, so it needs to
     # happen first.
     my $clause = $self->_special_charts;
 
@@ -1586,7 +1587,7 @@ sub _params_to_data_structure {
     
     # And then process the modern "custom search" format.
     $clause->add( $self->_custom_search );
-   
+    
     return $clause;
 }
 
@@ -1617,7 +1618,7 @@ sub _boolean_charts {
                 my $identifier = "$chart_id-$and_id-$or_id";
                 my $field = $params->{"field$identifier"};
                 my $operator = $params->{"type$identifier"};
-                my $value = $params->{"value$identifier"};                
+                my $value = $params->{"value$identifier"};
                 $or_clause->add($field, $operator, $value);
             }
             $and_clause->add($or_clause);
@@ -1636,13 +1637,19 @@ sub _custom_search {
     my @field_ids = $self->_field_ids;
     return unless scalar @field_ids;
 
-    my $current_clause = new Bugzilla::Search::Clause($params->{j_top});
+    my $joiner = $params->{j_top} || '';
+    my $current_clause = $joiner eq 'AND_G'
+        ? new Bugzilla::Search::ClauseGroup()
+        : new Bugzilla::Search::Clause($joiner);
+
     my @clause_stack;
     foreach my $id (@field_ids) {
         my $field = $params->{"f$id"};
         if ($field eq 'OP') {
-            my $joiner = $params->{"j$id"};
-            my $new_clause = new Bugzilla::Search::Clause($joiner);
+            my $joiner = $params->{"j$id"} || '';
+            my $new_clause = $joiner eq 'AND_G'
+                ? new Bugzilla::Search::ClauseGroup()
+                : new Bugzilla::Search::Clause($joiner);
             $new_clause->negate($params->{"n$id"});
             $current_clause->add($new_clause);
             push(@clause_stack, $current_clause);
@@ -1681,14 +1688,12 @@ sub _field_ids {
 }
 
 sub _handle_chart {
-    my ($self, $chart_id, $condition) = @_;
+    my ($self, $chart_id, $clause, $condition) = @_;
     my $dbh = Bugzilla->dbh;
     my $params = $self->_params;
     my ($field, $operator, $value) = $condition->fov;
-
-    $field = FIELD_MAP->{$field} || $field;
-
     return if (!defined $field or !defined $operator or !defined $value);
+    $field = FIELD_MAP->{$field} || $field;
     
     my $string_value;
     if (ref $value eq 'ARRAY') {
@@ -1719,15 +1724,19 @@ sub _handle_chart {
     # on multiple values, like anyexact.
     
     my %search_args = (
-        chart_id   => $chart_id,
-        sequence   => $chart_id,
-        field      => $field,
-        full_field => $full_field,
-        operator   => $operator,
-        value      => $string_value,
-        all_values => $value,
-        joins      => [],
+        chart_id     => $chart_id,
+        sequence     => $chart_id,
+        field        => $field,
+        full_field   => $full_field,
+        operator     => $operator,
+        value        => $string_value,
+        all_values   => $value,
+        joins        => [],
+        bugs_table   => 'bugs',
+        table_suffix => '',
     );
+    $clause->update_search_args(\%search_args);
+
     $search_args{quoted} = $self->_quote_unless_numeric(\%search_args);
     # This should add a "term" selement to %search_args.
     $self->do_search_function(\%search_args);
@@ -1743,7 +1752,12 @@ sub _handle_chart {
         field => $field, type => $operator,
         value => $string_value, term => $search_args{term},
     });
-    
+
+    foreach my $join (@{ $search_args{joins} }) {
+        $join->{bugs_table}   = $search_args{bugs_table};
+        $join->{table_suffix} = $search_args{table_suffix};
+    }
+
     $condition->translated(\%search_args);
 }
 
@@ -1899,8 +1913,14 @@ sub _quote_unless_numeric {
 }
 
 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);
 }
 
 # Used by anyexact to get the list of input values. This allows us to
@@ -2653,8 +2673,7 @@ sub _multiselect_term {
     my $term = $args->{term};
     $term .= $args->{_extra_where} || '';
     my $select = $args->{_select_field} || 'bug_id';
-    my $not_sql = $not ? "NOT " : '';
-    return "bugs.bug_id ${not_sql}IN (SELECT $select FROM $table WHERE $term)";
+    return build_subselect("$args->{bugs_table}.bug_id", $select, $table, $term, $not);
 }
 
 ###############################
diff --git a/Bugzilla/Search/Clause.pm b/Bugzilla/Search/Clause.pm
index ff50cb944..77969440e 100644
--- a/Bugzilla/Search/Clause.pm
+++ b/Bugzilla/Search/Clause.pm
@@ -30,6 +30,11 @@ sub children {
     return $self->{children};
 }
 
+sub update_search_args {
+    my ($self, $search_args) = @_;
+    # abstract
+}
+
 sub joiner { return $_[0]->{joiner} }
 
 sub has_translated_conditions {
@@ -71,7 +76,7 @@ sub walk_conditions {
     my ($self, $callback) = @_;
     foreach my $child (@{ $self->children }) {
         if ($child->isa('Bugzilla::Search::Condition')) {
-            $callback->($child);
+            $callback->($self, $child);
         }
         else {
             $child->walk_conditions($callback);
diff --git a/Bugzilla/Search/ClauseGroup.pm b/Bugzilla/Search/ClauseGroup.pm
new file mode 100644
index 000000000..98c9779c9
--- /dev/null
+++ b/Bugzilla/Search/ClauseGroup.pm
@@ -0,0 +1,97 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Search::ClauseGroup;
+
+use 5.10.1;
+use strict;
+
+use base qw(Bugzilla::Search::Clause);
+
+use Bugzilla::Error;
+use Bugzilla::Search::Condition qw(condition);
+use Bugzilla::Util qw(trick_taint);
+use List::MoreUtils qw(uniq);
+
+use constant UNSUPPORTED_FIELDS => qw(
+    attach_data.thedata
+    classification
+    commenter
+    component
+    longdescs.count
+    product
+    owner_idle_time
+);
+
+sub new {
+    my ($class) = @_;
+    my $self = bless({ joiner => 'AND' }, $class);
+    # Add a join back to the bugs table which will be used to group conditions
+    # for this clause
+    my $condition = Bugzilla::Search::Condition->new({});
+    $condition->translated({
+        joins => [{
+            table => 'bugs',
+            as    => 'bugs_g0',
+            from  => 'bug_id',
+            to    => 'bug_id',
+            extra => [],
+        }],
+        term => '1 = 1',
+    });
+    $self->SUPER::add($condition);
+    $self->{group_condition} = $condition;
+    return $self;
+}
+
+sub add {
+    my ($self, @args) = @_;
+    my $field = scalar(@args) == 3 ? $args[0] : $args[0]->{field};
+
+    # We don't support nesting of conditions under this clause
+    if (scalar(@args) == 1 && !$args[0]->isa('Bugzilla::Search::Condition')) {
+        ThrowUserError('search_grouped_invalid_nesting');
+    }
+
+    # Ensure all conditions use the same field
+    if (!$self->{_field}) {
+        $self->{_field} = $field;
+    } elsif ($field ne $self->{_field}) {
+        ThrowUserError('search_grouped_field_mismatch');
+    }
+
+    # Unsupported fields
+    if (grep { $_ eq $field } UNSUPPORTED_FIELDS ) {
+        ThrowUserError('search_grouped_field_invalid', { field => $field });
+    }
+
+    $self->SUPER::add(@args);
+}
+
+sub update_search_args {
+    my ($self, $search_args) = @_;
+
+    # No need to change things if there's only one child condition
+    return unless scalar(@{ $self->children }) > 1;
+
+    # we want all the terms to use the same join table
+    if (!exists $self->{_first_chart_id}) {
+        $self->{_first_chart_id} = $search_args->{chart_id};
+    } else {
+        $search_args->{chart_id} = $self->{_first_chart_id};
+    }
+
+    my $suffix = '_g' . $self->{_first_chart_id};
+    $self->{group_condition}->{translated}->{joins}->[0]->{as} = "bugs$suffix";
+
+    $search_args->{full_field} =~ s/^bugs\./bugs$suffix\./;
+
+    $search_args->{table_suffix} = $suffix;
+    $search_args->{bugs_table} = "bugs$suffix";
+}
+
+1;