From da12cec61f8c7e667b00fc5fc39c827d3593f021 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Thu, 22 Nov 2012 16:23:53 +0800 Subject: Bug 780820: Allows for multiple custom search criteria to match one field r=dkl,a=LpSolit --- Bugzilla/DB.pm | 6 ++- Bugzilla/DB/Oracle.pm | 6 +-- Bugzilla/Search.pm | 71 ++++++++++++++++++++----------- Bugzilla/Search/Clause.pm | 7 ++- Bugzilla/Search/ClauseGroup.pm | 97 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 32 deletions(-) create mode 100644 Bugzilla/Search/ClauseGroup.pm (limited to 'Bugzilla') 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; -- cgit v1.2.3-24-g4f1b