diff options
| -rw-r--r-- | Bugzilla/Search.pm | 178 | ||||
| -rwxr-xr-x | buglist.cgi | 47 | ||||
| -rwxr-xr-x | collectstats.pl | 3 | ||||
| -rw-r--r-- | extensions/BMO/Extension.pm | 3 | ||||
| -rw-r--r-- | extensions/MyDashboard/lib/Queries.pm | 7 | ||||
| -rwxr-xr-x | report.cgi | 8 | ||||
| -rw-r--r-- | template/en/default/list/list.html.tmpl | 11 | ||||
| -rw-r--r-- | template/en/default/reports/report.html.tmpl | 4 | ||||
| -rwxr-xr-x | whine.pl | 9 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/FieldTest.pm | 11 |
10 files changed, 59 insertions, 222 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 5fb2352a5..0cf618a4c 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -57,10 +57,9 @@ use Data::Dumper; use Date::Format; use Date::Parse; use Scalar::Util qw(blessed); -use List::MoreUtils qw(all firstidx part uniq); +use List::MoreUtils qw(all part uniq); use POSIX qw(INT_MAX); use Storable qw(dclone); -use Time::HiRes qw(gettimeofday tv_interval); # Description Of Boolean Charts # ----------------------------- @@ -709,70 +708,7 @@ sub new { # Public Accessors # #################### -sub data { - my $self = shift; - return $self->{data} if $self->{data}; - my $dbh = Bugzilla->dbh; - - # If all fields belong to the 'bugs' table, there is no need to split - # the original query into two pieces. Else we override the 'fields' - # argument to first get bug IDs based on the search criteria defined - # by the caller, and the desired fields are collected in the 2nd query. - my @orig_fields = $self->_input_columns; - my $all_in_bugs_table = 1; - foreach my $field (@orig_fields) { - next if $self->COLUMNS->{$field}->{name} =~ /^bugs\.\w+$/; - $self->{fields} = ['bug_id']; - $all_in_bugs_table = 0; - last; - } - - my $start_time = [gettimeofday()]; - my $sql = $self->_sql; - # Do we just want bug IDs to pass to the 2nd query or all the data immediately? - my $func = $all_in_bugs_table ? 'selectall_arrayref' : 'selectcol_arrayref'; - my $bug_ids = $dbh->$func($sql); - my @extra_data = ({sql => $sql, time => tv_interval($start_time)}); - # Restore the original 'fields' argument, just in case. - $self->{fields} = \@orig_fields unless $all_in_bugs_table; - - # If there are no bugs found, or all fields are in the 'bugs' table, - # there is no need for another query. - if (!scalar @$bug_ids || $all_in_bugs_table) { - $self->{data} = $bug_ids; - return wantarray ? ($self->{data}, \@extra_data) : $self->{data}; - } - - # Make sure the bug_id will be returned. If not, append it to the list. - my $pos = firstidx { $_ eq 'bug_id' } @orig_fields; - if ($pos < 0) { - push(@orig_fields, 'bug_id'); - $pos = $#orig_fields; - } - - # Now create a query with the buglist above as the single criteria - # and the fields that the caller wants. No need to redo security checks; - # the list has already been validated above. - my $search = $self->new('fields' => \@orig_fields, - 'params' => {bug_id => $bug_ids, bug_id_type => 'anyexact'}, - 'sharer' => $self->_sharer_id, - 'user' => $self->_user, - 'allow_unlimited' => 1, - '_no_security_check' => 1); - - $start_time = [gettimeofday()]; - $sql = $search->_sql; - my $unsorted_data = $dbh->selectall_arrayref($sql); - push(@extra_data, {sql => $sql, time => tv_interval($start_time)}); - # Let's sort the data. We didn't do it in the query itself because - # we already know in which order to sort bugs thanks to the first query, - # and this avoids additional table joins in the SQL query. - my %data = map { $_->[$pos] => $_ } @$unsorted_data; - $self->{data} = [map { $data{$_} } @$bug_ids]; - return wantarray ? ($self->{data}, \@extra_data) : $self->{data}; -} - -sub _sql { +sub sql { my ($self) = @_; return $self->{sql} if $self->{sql}; my $dbh = Bugzilla->dbh; @@ -806,7 +742,7 @@ sub search_description { # Make sure that the description has actually been generated if # people are asking for the whole thing. else { - $self->_sql; + $self->sql; } return $self->{'search_description'}; } @@ -1164,7 +1100,6 @@ sub _standard_joins { my ($self) = @_; my $user = $self->_user; my @joins; - return () if $self->{_no_security_check}; my $security_join = { table => 'bug_group_map', @@ -1241,7 +1176,6 @@ sub _translate_join { # group security. sub _standard_where { my ($self) = @_; - return ('1=1') if $self->{_no_security_check}; # If replication lags badly between the shadow db and the main DB, # it's possible for bugs to show up in searches before their group # controls are properly set. To prevent this, when initially creating @@ -3086,109 +3020,3 @@ sub translate_old_column { } 1; - -__END__ - -=head1 NAME - -Bugzilla::Search - Provides methods to run queries against bugs. - -=head1 SYNOPSIS - - use Bugzilla::Search; - - my $search = new Bugzilla::Search({'fields' => \@fields, - 'params' => \%search_criteria, - 'sharer' => $sharer_id, - 'user' => $user_obj, - 'allow_unlimited' => 1}); - - my $data = $search->data; - my ($data, $extra_data) = $search->data; - -=head1 DESCRIPTION - -Search.pm represents a search object. It's the single way to collect -data about bugs in a secure way. The list of bugs matching criteria -defined by the caller are filtered based on the user privileges. - -=head1 METHODS - -=head2 new - -=over - -=item B<Description> - -Create a Bugzilla::Search object. - -=item B<Params> - -=over - -=item C<fields> - -An arrayref representing the bug attributes for which data is desired. -Legal attributes are listed in the fielddefs DB table. At least one field -must be defined, typically the 'bug_id' field. - -=item C<params> - -A hashref representing search criteria. Each key => value pair represents -a search criteria, where the key is the search field and the value is the -value for this field. At least one search criteria must be defined if the -'search_allow_no_criteria' parameter is turned off, else an error is thrown. - -=item C<sharer> - -When a saved search is shared by a user, this is his user ID. - -=item C<user> - -A L<Bugzilla::User> object representing the user to whom the data is addressed. -All security checks are done based on this user object, so it's not safe -to share results of the query with other users as not all users have the -same privileges or have the same role for all bugs in the list. If this -parameter is not defined, then the currently logged in user is taken into -account. If no user is logged in, then only public bugs will be returned. - -=item C<allow_unlimited> - -If set to a true value, the number of bugs retrieved by the query is not -limited. - -=back - -=item B<Returns> - -A L<Bugzilla::Search> object. - -=back - -=head2 data - -=over - -=item B<Description> - -Returns bugs matching search criteria passed to C<new()>. - -=item B<Params> - -None - -=item B<Returns> - -In scalar context, this method returns a reference to a list of bugs. -Each item of the list represents a bug, which is itself a reference to -a list where each item represents a bug attribute, in the same order as -specified in the C<fields> parameter of C<new()>. - -In list context, this methods also returns a reference to a list containing -references to hashes. For each hash, two keys are defined: C<sql> contains -the SQL query which has been executed, and C<time> contains the time spent -to execute the SQL query, in seconds. There can be either a single hash, or -two hashes if two SQL queries have been executed sequentially to get all the -required data. - -=back diff --git a/buglist.cgi b/buglist.cgi index 2d81ed56d..517aa4507 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -51,6 +51,7 @@ use Bugzilla::Status; use Bugzilla::Token; use Date::Parse; +use Time::HiRes qw(gettimeofday tv_interval); my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; @@ -775,6 +776,8 @@ my $search = new Bugzilla::Search('fields' => \@selectcolumns, 'params' => scalar $params->Vars, 'order' => \@orderstrings, 'sharer' => $sharer_id); +my $query = $search->sql; +$vars->{'search_description'} = $search->search_description; # We don't want saved searches and other buglist things to save # our default limit. @@ -784,6 +787,21 @@ $params->delete('limit') if $vars->{'default_limited'}; # Query Execution ################################################################################ +if ($cgi->param('debug') + && Bugzilla->params->{debug_group} + && $user->in_group(Bugzilla->params->{debug_group}) +) { + $vars->{'debug'} = 1; + $vars->{'query'} = $query; + # Explains are limited to admins because you could use them to figure + # out how many hidden bugs are in a particular product (by doing + # searches and looking at the number of rows the explain says it's + # examining). + if (Bugzilla->user->in_group('admin')) { + $vars->{'query_explain'} = $dbh->bz_explain($query); + } +} + # Time to use server push to display an interim message to the user until # the query completes and we can display the bug list. if ($serverpush) { @@ -816,28 +834,11 @@ $::SIG{TERM} = 'DEFAULT'; $::SIG{PIPE} = 'DEFAULT'; # Execute the query. -my ($data, $extra_data) = $search->data; -$vars->{'search_description'} = $search->search_description; +my $start_time = [gettimeofday()]; +my $buglist_sth = $dbh->prepare($query); +$buglist_sth->execute(); +$vars->{query_time} = tv_interval($start_time); -if ($cgi->param('debug') - && Bugzilla->params->{debug_group} - && $user->in_group(Bugzilla->params->{debug_group}) -) { - $vars->{'debug'} = 1; - $vars->{'queries'} = $extra_data; - my $query_time = 0; - $query_time += $_->{'time'} foreach @$extra_data; - $vars->{'query_time'} = $query_time; - # Explains are limited to admins because you could use them to figure - # out how many hidden bugs are in a particular product (by doing - # searches and looking at the number of rows the explain says it's - # examining). - if ($user->in_group('admin')) { - foreach my $query (@$extra_data) { - $query->{explain} = $dbh->bz_explain($query->{sql}); - } - } -} ################################################################################ # Results Retrieval @@ -869,14 +870,14 @@ my @bugidlist; my @bugs; # the list of records -foreach my $row (@$data) { +while (my @row = $buglist_sth->fetchrow_array()) { my $bug = {}; # a record # Slurp the row of data into the record. # The second from last column in the record is the number of groups # to which the bug is restricted. foreach my $column (@selectcolumns) { - $bug->{$column} = shift @$row; + $bug->{$column} = shift @row; } # Process certain values further (i.e. date format conversion). diff --git a/collectstats.pl b/collectstats.pl index c5db30b5f..1487e5a72 100755 --- a/collectstats.pl +++ b/collectstats.pl @@ -504,7 +504,8 @@ sub CollectSeriesData { 'fields' => ["bug_id"], 'allow_unlimited' => 1, 'user' => $user); - $data = $search->data; + my $sql = $search->sql; + $data = $shadow_dbh->selectall_arrayref($sql); }; if (!$@) { diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index 7909d732d..554fb2ef2 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -796,6 +796,9 @@ sub search_operator_field_override { my @comments = $cgi->param('comments'); my $exclude_comments = scalar(@comments) && !grep { $_ eq '1' } @comments; + use Data::Dumper; + print STDERR Dumper \@comments; + if ($cgi->param('query_format') && $cgi->param('query_format') eq 'specific' && $exclude_comments diff --git a/extensions/MyDashboard/lib/Queries.pm b/extensions/MyDashboard/lib/Queries.pm index 2ec4c8a74..e81e7f73f 100644 --- a/extensions/MyDashboard/lib/Queries.pm +++ b/extensions/MyDashboard/lib/Queries.pm @@ -145,10 +145,13 @@ sub query_bugs { params => scalar $params->Vars, order => [ QUERY_ORDER ]); - my $data = $search->data; + my $query = $search->sql(); + my $sth = $dbh->prepare($query); + $sth->execute(); + my $rows = $sth->fetchall_arrayref(); my @bugs; - foreach my $row (@$data) { + foreach my $row (@$rows) { my $bug = {}; foreach my $column (SELECT_COLUMNS) { $bug->{$column} = shift @$row; diff --git a/report.cgi b/report.cgi index decbaeeb2..7bff62be9 100755 --- a/report.cgi +++ b/report.cgi @@ -131,12 +131,13 @@ my $search = new Bugzilla::Search( params => scalar $params->Vars, allow_unlimited => 1, ); +my $query = $search->sql; $::SIG{TERM} = 'DEFAULT'; $::SIG{PIPE} = 'DEFAULT'; -Bugzilla->switch_to_shadow_db(); -my ($results, $extra_data) = $search->data; +my $dbh = Bugzilla->switch_to_shadow_db(); +my $results = $dbh->selectall_arrayref($query); # We have a hash of hashes for the data itself, and a hash to hold the # row/col/table names. @@ -223,7 +224,8 @@ if ($width && $formatparam eq "bar") { $vars->{'width'} = $width if $width; $vars->{'height'} = $height if $height; -$vars->{'queries'} = $extra_data; + +$vars->{'query'} = $query; if ($cgi->param('debug') && Bugzilla->params->{debug_group} diff --git a/template/en/default/list/list.html.tmpl b/template/en/default/list/list.html.tmpl index cda06ac21..055cd2e8c 100644 --- a/template/en/default/list/list.html.tmpl +++ b/template/en/default/list/list.html.tmpl @@ -60,13 +60,10 @@ [% IF debug %] <div class="bz_query_debug"> - <p>Total execution time: [% query_time FILTER html %] seconds</p> - [% FOREACH query = queries %] - <p>[% query.sql FILTER html %]</p> - <p>Execution time: [% query.time FILTER html %] seconds</p> - [% IF query.explain %] - <pre>[% query.explain FILTER html %]</pre> - [% END %] + <p>[% query FILTER html %]</p> + <p>Execution time: [% query_time FILTER html %] seconds</p> + [% IF query_explain.defined %] + <pre>[% query_explain FILTER html %]</pre> [% END %] </div> [% END %] diff --git a/template/en/default/reports/report.html.tmpl b/template/en/default/reports/report.html.tmpl index 38b64df0b..94725ae81 100644 --- a/template/en/default/reports/report.html.tmpl +++ b/template/en/default/reports/report.html.tmpl @@ -81,9 +81,7 @@ %] [% IF debug %] - [% FOREACH query = queries %] - <p>[% query.sql FILTER html %]</p> - [% END %] + <p>[% query FILTER html %]</p> [% END %] <div align="center"> @@ -453,7 +453,7 @@ sub run_queries { 'user' => $args->{'recipient'}, # the search runs as the recipient ); # If a query fails for whatever reason, it shouldn't kill the script. - my $data = eval { $search->data }; + my $sqlquery = eval { $search->sql }; if ($@) { print STDERR get_text('whine_query_failed', { query_name => $thisquery->{'name'}, author => $args->{'author'}, @@ -461,12 +461,15 @@ sub run_queries { next; } - foreach my $row (@$data) { + $sth = $dbh->prepare($sqlquery); + $sth->execute; + + while (my @row = $sth->fetchrow_array) { my $bug = {}; for my $field (@searchfields) { my $fieldname = $field; $fieldname =~ s/^bugs\.//; # No need for bugs.whatever - $bug->{$fieldname} = shift @$row; + $bug->{$fieldname} = shift @row; } if ($thisquery->{'onemailperbug'}) { diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm index bd5fd905a..283a90d16 100644 --- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm +++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm @@ -544,13 +544,13 @@ sub do_tests { my $sql; TODO: { local $TODO = $search_broken if $search_broken; - lives_ok { $sql = $search->_sql } "$name: generate SQL"; + lives_ok { $sql = $search->sql } "$name: generate SQL"; } my $results; SKIP: { skip "Can't run SQL without any SQL", 1 if !defined $sql; - $results = $self->_test_sql($search); + $results = $self->_test_sql($sql); } $self->_test_content($results, $sql); @@ -567,11 +567,12 @@ sub _test_search_object_creation { } sub _test_sql { - my ($self, $search) = @_; + my ($self, $sql) = @_; + my $dbh = Bugzilla->dbh; my $name = $self->name; my $results; - lives_ok { $results = $search->data } "$name: Run SQL Query" - or diag($search->_sql); + lives_ok { $results = $dbh->selectall_arrayref($sql) } "$name: Run SQL Query" + or diag($sql); return $results; } |