summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Bugzilla/DB.pm6
-rw-r--r--Bugzilla/DB/Oracle.pm6
-rw-r--r--Bugzilla/Search.pm126
-rw-r--r--Bugzilla/Search/Clause.pm36
-rw-r--r--Bugzilla/Search/Condition.pm9
-rw-r--r--Bugzilla/Search/Quicksearch.pm11
-rw-r--r--xt/lib/Bugzilla/Test/Search/Constants.pm83
-rw-r--r--xt/lib/Bugzilla/Test/Search/FieldTest.pm32
8 files changed, 251 insertions, 58 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;
}
diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm
index 512d180d9..051570ff8 100644
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -197,11 +197,14 @@ use constant GREATERTHAN_BROKEN => (
);
# allwords and allwordssubstr have these broken tests in common.
-#
-# allwordssubstr on cc fields matches against a single cc,
-# instead of matching against all ccs on a bug.
use constant ALLWORDS_BROKEN => (
+ # allwordssubstr on cc fields matches against a single cc,
+ # instead of matching against all ccs on a bug.
cc => { contains => [1] },
+ # bug 828344 changed how these searches operate to revert back to the 4.0
+ # behavour, so these tests need to be updated (bug 849117).
+ 'flagtypes.name' => { contains => [1] },
+ longdesc => { contains => [1] },
);
# Fields that don't generally work at all with changed* searches, but
@@ -330,6 +333,24 @@ use constant KNOWN_BROKEN => {
# This should probably search the reporter.
creation_ts => { contains => [1] },
},
+ notequals => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ longdesc => { contains => [1] },
+ },
+ notregexp => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ longdesc => { contains => [1] },
+ },
+ notsubstring => {
+ 'flagtypes.name' => { contains => [5] },
+ longdesc => { contains => [1] },
+ },
+ nowords => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ },
+ nowordssubstr => {
+ 'flagtypes.name' => { contains => [5] },
+ },
};
###################
@@ -360,17 +381,34 @@ use constant CHANGED_FROM_TO_BROKEN_NOT => (
# These are field/operator combinations that are broken when run under NOT().
use constant BROKEN_NOT => {
- allwords => {
- cc => { contains => [1] },
+ allwords => {
+ cc => { contains => [1] },
+ 'flagtypes.name' => { contains => [1, 5] },
+ longdesc => { contains => [1] },
},
'allwords-<1> <2>' => {
cc => { },
},
allwordssubstr => {
- cc => { contains => [1] },
+ cc => { contains => [1] },
+ 'flagtypes.name' => { contains => [5, 6] },
+ longdesc => { contains => [1] },
},
'allwordssubstr-<1>,<2>' => {
- cc => { },
+ cc => { },
+ longdesc => { contains => [1] },
+ },
+ anyexact => {
+ 'flagtypes.name' => { contains => [1, 2, 5] },
+ },
+ anywords => {
+ 'flagtypes.name' => { contains => [1, 2, 5] },
+ },
+ anywordssubstr => {
+ 'flagtypes.name' => { contains => [5] },
+ },
+ casesubstring => {
+ 'flagtypes.name' => { contains => [5] },
},
changedafter => {
"attach_data.thedata" => { contains => [2, 3, 4] },
@@ -397,7 +435,6 @@ use constant BROKEN_NOT => {
dependson => { contains => [1, 3] },
work_time => { contains => [1] },
FIELD_TYPE_BUG_ID, { contains => [1 .. 4] },
-
},
changedto => {
CHANGED_BROKEN_NOT,
@@ -406,10 +443,38 @@ use constant BROKEN_NOT => {
"remaining_time" => { contains => [1] },
},
greaterthan => {
- cc => { contains => [1] },
+ cc => { contains => [1] },
+ 'flagtypes.name' => { contains => [5] },
},
greaterthaneq => {
cc => { contains => [1] },
+ 'flagtypes.name' => { contains => [2, 5] },
+ },
+ equals => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ },
+ notequals => {
+ longdesc => { contains => [1] },
+ },
+ notregexp => {
+ longdesc => { contains => [1] },
+ },
+ notsubstring => {
+ longdesc => { contains => [1] },
+ },
+ lessthan => {
+ 'flagtypes.name' => { contains => [5] },
+ },
+ lessthaneq => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ },
+ regexp => {
+ 'flagtypes.name' => { contains => [1, 5] },
+ longdesc => { contains => [1] },
+ },
+ substring => {
+ 'flagtypes.name' => { contains => [5] },
+ longdesc => { contains => [1] },
},
};
diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
index 283a90d16..ee25f2dc6 100644
--- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm
+++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
@@ -28,6 +28,7 @@ use strict;
use warnings;
use Bugzilla::Search;
use Bugzilla::Test::Search::Constants;
+use Bugzilla::Util qw(trim);
use Data::Dumper;
use Scalar::Util qw(blessed);
@@ -72,6 +73,13 @@ sub bug {
my $self = shift;
return $self->search_test->bug(@_);
}
+sub number {
+ my ($self, $id) = @_;
+ foreach my $number (1..NUM_BUGS) {
+ return $number if $self->search_test->bug($number)->id == $id;
+ }
+ return 0;
+}
# The name displayed for this test by Test::More. Used in test descriptions.
sub name {
@@ -147,9 +155,18 @@ sub translated_value {
return $self->{translated_value};
}
# Used in failure diagnostic messages.
-sub debug_value {
- my ($self) = @_;
- return "Value: '" . $self->translated_value . "'";
+sub debug_fail {
+ my ($self, $number, $results, $sql) = @_;
+ my @expected = @{ $self->test->{contains} };
+ my @results = sort
+ map { $self->number($_) }
+ map { $_->[0] }
+ @$results;
+ return
+ " Value: '" . $self->translated_value . "'\n" .
+ "Expected: [" . join(',', @expected) . "]\n" .
+ " Results: [" . join(',', @results) . "]\n" .
+ trim($sql) . "\n";
}
# True for a bug if we ran the "transform" function on it and the
@@ -184,6 +201,7 @@ sub bug_is_contained {
# The tests we know are broken for this operator/field combination.
sub _known_broken {
my ($self, $constant, $skip_pg_check) = @_;
+
$constant ||= KNOWN_BROKEN;
my $field = $self->field;
my $type = $self->field_object->type;
@@ -192,8 +210,8 @@ sub _known_broken {
my $value_name = "$operator-$value";
if (my $extra_name = $self->test->{extra_name}) {
$value_name .= "-$extra_name";
- }
-
+ }
+
my $value_broken = $constant->{$value_name}->{$field};
$value_broken ||= $constant->{$value_name}->{$type};
return $value_broken if $value_broken;
@@ -601,12 +619,12 @@ sub _test_content_for_bug {
if ($self->bug_is_contained($number)) {
ok($result_ids{$bug_id},
"$name: contains bug $number ($bug_id)")
- or diag Dumper($results) . $self->debug_value . "\n\nSQL: $sql";
+ or diag $self->debug_fail($number, $results, $sql);
}
else {
ok(!$result_ids{$bug_id},
"$name: does not contain bug $number ($bug_id)")
- or diag Dumper($results) . $self->debug_value . "\n\nSQL: $sql";
+ or diag $self->debug_fail($number, $results, $sql);
}
}
}