Reverted Bug 836067 due to buglist crashing and do not yet know the reason

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">
diff --git a/whine.pl b/whine.pl
index e6161cfeb..ad6067228 100755
--- a/whine.pl
+++ b/whine.pl
@@ -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;
 }