Bug 828344: "contains all of the words" no longer looks for all words within the same comment or flag

r=LpSolit, a=LpSolit

6 files changed, 152 insertions, 42 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 0c841632f..1f9c31518 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -405,8 +405,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 1427bcedd..20eb0e550 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -216,16 +216,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 656d163ea..8e419c0ee 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -290,12 +290,14 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     },
     dependson        => MULTI_SELECT_OVERRIDE,
     keywords         => MULTI_SELECT_OVERRIDE,
-    'flagtypes.name' => MULTI_SELECT_OVERRIDE,
+    'flagtypes.name' => {
+        _non_changed => \&_flagtypes_nonchanged,
+    },
     longdesc => {
-        %{ MULTI_SELECT_OVERRIDE() },
         changedby     => \&_long_desc_changedby,
         changedbefore => \&_long_desc_changedbefore_after,
         changedafter  => \&_long_desc_changedbefore_after,
+        _non_changed  => \&_long_desc_nonchanged,
     },
     'longdescs.count' => {
         changedby     => \&_long_desc_changedby,
@@ -690,8 +692,16 @@ sub sql {
     my ($self) = @_;
     return $self->{sql} if $self->{sql};
     my $dbh = Bugzilla->dbh;
-    
+
     my ($joins, $clause) = $self->_charts_to_conditions();
+
+    if (!$clause->as_string
+        && !Bugzilla->params->{'search_allow_no_criteria'}
+        && !$self->{allow_unlimited})
+    {
+        ThrowUserError('buglist_parameters_required');
+    }
+
     my $select = join(', ', $self->_sql_select);
     my $from = $self->_sql_from($joins);
     my $where = $self->_sql_where($clause);
@@ -1191,14 +1201,7 @@ sub _sql_where {
     # SQL a bit more readable for debugging.
     my $where = join("\n   AND ", $self->_standard_where);
     my $clause_sql = $main_clause->as_string;
-    if ($clause_sql) {
-        $where .= "\n   AND " . $clause_sql;
-    }
-    elsif (!Bugzilla->params->{'search_allow_no_criteria'}
-           && !$self->{allow_unlimited})
-    {
-        ThrowUserError('buglist_parameters_required');
-    }
+    $where .= "\n   AND " . $clause_sql if $clause_sql;
     return $where;
 }
 
@@ -1689,6 +1692,7 @@ sub _handle_chart {
         value      => $string_value,
         all_values => $value,
         joins      => [],
+        condition  => $condition,
     );
     $search_args{quoted} = $self->_quote_unless_numeric(\%search_args);
     # This should add a "term" selement to %search_args.
@@ -1861,8 +1865,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
@@ -2327,14 +2337,50 @@ sub _long_desc_changedbefore_after {
     }
 }
 
+sub _long_desc_nonchanged {
+    my ($self, $args) = @_;
+    my ($chart_id, $operator, $value, $joins) =
+        @$args{qw(chart_id operator value joins)};
+    my $dbh = Bugzilla->dbh;
+
+    my $table = "longdescs_$chart_id";
+    my $join_args = {
+        chart_id   => $chart_id,
+        sequence   => $chart_id,
+        field      => 'longdesc',
+        full_field => "$table.thetext",
+        operator   => $operator,
+        value      => $value,
+        all_values => $value,
+        quoted     => $dbh->quote($value),
+        joins      => [],
+    };
+    $self->_do_operator_function($join_args);
+
+    # If the user is not part of the insiders group, they cannot see
+    # private comments
+    if (!$self->_user->is_insider) {
+        $join_args->{term} .= " AND $table.isprivate = 0";
+    }
+
+    my $join = {
+        table => 'longdescs',
+        as    => $table,
+        extra => [ $join_args->{term} ],
+    };
+    push(@$joins, $join);
+
+    $args->{term} =  "$table.comment_id IS NOT NULL";
+}
+
 sub _content_matches {
     my ($self, $args) = @_;
     my ($chart_id, $joins, $fields, $operator, $value) =
         @$args{qw(chart_id joins fields operator value)};
     my $dbh = Bugzilla->dbh;
-    
+
     # "content" is an alias for columns containing text for which we
-    # can search a full-text index and retrieve results by relevance, 
+    # can search a full-text index and retrieve results by relevance,
     # currently just bug comments (and summaries to some degree).
     # There's only one way to search a full-text index, so we only
     # accept the "matches" operator, which is specific to full-text
@@ -2582,6 +2628,53 @@ sub _multiselect_multiple {
     }
 }
 
+sub _flagtypes_nonchanged {
+    my ($self, $args) = @_;
+    my ($chart_id, $operator, $value, $joins, $condition) =
+        @$args{qw(chart_id operator value joins condition)};
+    my $dbh = Bugzilla->dbh;
+
+    # For 'not' operators, we need to negate the whole term.
+    # If you search for "Flags" (does not contain) "approval+" we actually want
+    # to return *bugs* that don't contain an approval+ flag.  Without rewriting
+    # the negation we'll search for *flags* which don't contain approval+.
+    if ($operator =~ s/^not//) {
+        $args->{operator} = $operator;
+        $condition->operator($operator);
+        $condition->negate(1);
+    }
+
+    my $subselect_args = {
+        chart_id   => $chart_id,
+        sequence   => $chart_id,
+        field      => 'flagtypes.name',
+        full_field =>  $dbh->sql_string_concat("flagtypes_$chart_id.name", "flags_$chart_id.status"),
+        operator   => $operator,
+        value      => $value,
+        all_values => $value,
+        quoted     => $dbh->quote($value),
+        joins      => [],
+    };
+    $self->_do_operator_function($subselect_args);
+    my $subselect_term = $subselect_args->{term};
+
+    # don't call build_subselect as this must run as a true sub-select
+    $args->{term} = "EXISTS (
+        SELECT 1
+          FROM bugs bugs_$chart_id
+          LEFT JOIN attachments AS attachments_$chart_id
+                    ON bugs_$chart_id.bug_id = attachments_$chart_id.bug_id
+          LEFT JOIN flags AS flags_$chart_id
+                    ON bugs_$chart_id.bug_id = flags_$chart_id.bug_id
+                       AND (flags_$chart_id.attach_id = attachments_$chart_id.attach_id
+                            OR flags_$chart_id.attach_id IS NULL)
+          LEFT JOIN flagtypes AS flagtypes_$chart_id
+                    ON flags_$chart_id.type_id = flagtypes_$chart_id.id
+     WHERE bugs_$chart_id.bug_id = bugs.bug_id
+           AND $subselect_term
+    )";
+}
+
 sub _multiselect_nonchanged {
     my ($self, $args) = @_;
     my ($chart_id, $joins, $field, $operator) =
@@ -2659,8 +2752,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("bugs.bug_id", $select, $table, $term, $not);
 }
 
 ###############################
diff --git a/Bugzilla/Search/Clause.pm b/Bugzilla/Search/Clause.pm
index a068ce5ed..5f5ea5b50 100644
--- a/Bugzilla/Search/Clause.pm
+++ b/Bugzilla/Search/Clause.pm
@@ -93,25 +93,29 @@ sub walk_conditions {
 
 sub as_string {
     my ($self) = @_;
-    my @strings;
-    foreach my $child (@{ $self->children }) {
-        next if $child->isa(__PACKAGE__) && !$child->has_translated_conditions;
-        next if $child->isa('Bugzilla::Search::Condition')
-                && !$child->translated;
+    if (!$self->{sql}) {
+        my @strings;
+        foreach my $child (@{ $self->children }) {
+            next if $child->isa(__PACKAGE__) && !$child->has_translated_conditions;
+            next if $child->isa('Bugzilla::Search::Condition')
+                    && !$child->translated;
 
-        my $string = $child->as_string;
-        if ($self->joiner eq 'AND') {
-            $string = "( $string )" if $string =~ /OR/;
-        }
-        else {
-            $string = "( $string )" if $string =~ /AND/;
+            my $string = $child->as_string;
+            next unless $string;
+            if ($self->joiner eq 'AND') {
+                $string = "( $string )" if $string =~ /OR/;
+            }
+            else {
+                $string = "( $string )" if $string =~ /AND/;
+            }
+            push(@strings, $string);
         }
-        push(@strings, $string);
+
+        my $sql = join(' ' . $self->joiner . ' ', @strings);
+        $sql = "NOT( $sql )" if $sql && $self->negate;
+        $self->{sql} = $sql;
     }
-    
-    my $sql = join(' ' . $self->joiner . ' ', @strings);
-    $sql = "NOT( $sql )" if $sql && $self->negate;
-    return $sql;
+    return $self->{sql};
 }
 
 # Search.pm converts URL parameters to Clause objects. This helps do the
diff --git a/Bugzilla/Search/Condition.pm b/Bugzilla/Search/Condition.pm
index 2268da197..167b4f01e 100644
--- a/Bugzilla/Search/Condition.pm
+++ b/Bugzilla/Search/Condition.pm
@@ -32,9 +32,16 @@ sub new {
 }
 
 sub field    { return $_[0]->{field}    }
-sub operator { return $_[0]->{operator} }
 sub value    { return $_[0]->{value}    }
 
+sub operator {
+    my ($self, $value) = @_;
+    if (@_ == 2) {
+        $self->{operator} = $value;
+    }
+    return $self->{operator};
+}
+
 sub fov {
     my ($self) = @_;
     return ($self->field, $self->operator, $self->value);
diff --git a/Bugzilla/Search/Quicksearch.pm b/Bugzilla/Search/Quicksearch.pm
index 7424f831f..fd9d796d1 100644
--- a/Bugzilla/Search/Quicksearch.pm
+++ b/Bugzilla/Search/Quicksearch.pm
@@ -377,9 +377,14 @@ sub _handle_field_names {
 
     # Flag and requestee shortcut
     if ($or_operand =~ /^(?:flag:)?([^\?]+\?)([^\?]*)$/) {
-        addChart('flagtypes.name', 'substring', $1, $negate);
-        $chart++; $and = $or = 0; # Next chart for boolean AND
-        addChart('requestees.login_name', 'substring', $2, $negate);
+        my ($flagtype, $requestee) = ($1, $2);
+        addChart('flagtypes.name', 'substring', $flagtype, $negate);
+        if ($requestee) {
+            # AND
+            $chart++;
+            $and = $or = 0;
+            addChart('requestees.login_name', 'substring', $requestee, $negate);
+        }
         return 1;
     }