From 0b1e410c81430711a602adc56e4fc7667d1c841e Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Tue, 21 May 2013 01:54:06 +0800 Subject: Bug 828344: "contains all of the words" no longer looks for all words within the same comment or flag r=LpSolit, a=LpSolit --- Bugzilla/DB.pm | 6 +- Bugzilla/DB/Oracle.pm | 6 +- Bugzilla/Search.pm | 126 +++++++++++++++++++++++++++++++++++------ Bugzilla/Search/Clause.pm | 36 ++++++------ Bugzilla/Search/Condition.pm | 9 ++- Bugzilla/Search/Quicksearch.pm | 11 +++- 6 files changed, 152 insertions(+), 42 deletions(-) (limited to 'Bugzilla') 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; } -- cgit v1.2.3-24-g4f1b