From 4b5b03e55429a6e1c531650aa5be00f624b2bfa2 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 16 Oct 2013 22:26:31 +0800 Subject: Bug 926109: Error when searching for many columns at once (MariaDB can only use 61 tables in a join) --- Bugzilla/Search.pm | 69 ++++++++++++++++++++++++++++++++++- buglist.cgi | 11 +----- extensions/TrackingFlags/Extension.pm | 4 ++ extensions/TrackingFlags/lib/Flag.pm | 11 ++++++ 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 51f08f558..fa2689c0f 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -739,11 +739,20 @@ sub data { 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; } + # BMO - tracking flags are not on the bugs table anymore + $self->_display_columns(); + if (@{ $self->{tracking_flags} }) { + $all_in_bugs_table = 0; + } + + if (!$all_in_bugs_table) { + $self->{fields} = ['bug_id']; + } + 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? @@ -789,6 +798,44 @@ sub data { # and this avoids additional table joins in the SQL query. my %data = map { $_->[$pos] => $_ } @$unsorted_data; $self->{data} = [map { $data{$_} } @$bug_ids]; + + # BMO - get tracking flags values, and insert into result + if (@{ $self->{tracking_flags} }) { + # read values + my $values; + $sql = " + SELECT bugs.bug_id, tracking_flags.name, tracking_flags_bugs.value + FROM bugs + LEFT JOIN tracking_flags_bugs ON tracking_flags_bugs.bug_id = bugs.bug_id + LEFT JOIN tracking_flags ON tracking_flags.id = tracking_flags_bugs.tracking_flag_id + WHERE " . $dbh->sql_in('bugs.bug_id', $bug_ids); + $start_time = [gettimeofday()]; + my $rows = $dbh->selectall_arrayref($sql); + push(@extra_data, {sql => $sql, time => tv_interval($start_time)}); + foreach my $row (@$rows) { + $values->{$row->[0]}{$row->[1]} = $row->[2] if defined($row->[2]); + } + + # find the columns of the tracking flags + my %tf_pos; + for (my $i = 0; $i <= $#orig_fields; $i++) { + if (grep { $_ eq $orig_fields[$i] } @{ $self->{tracking_flags} }) { + $tf_pos{$orig_fields[$i]} = $i; + } + } + + # replace the placeholder value with the field's value + foreach my $row (@{ $self->{data} }) { + my $bug_id = $row->[$pos]; + next unless exists $values->{$bug_id}; + foreach my $field (keys %{ $values->{$bug_id} }) { + if (exists $tf_pos{$field}) { + $row->[$tf_pos{$field}] = $values->{$bug_id}{$field}; + } + } + } + } + return wantarray ? ($self->{data}, \@extra_data) : $self->{data}; } @@ -953,6 +1000,23 @@ sub _display_columns { # expects to get them back in the exact same order. my @columns = $self->_input_columns; + # BMO - to avoid massive amounts of joins, if we're selecting a lot of + # tracking flags, replace them with placeholders. the values will be + # retrieved later and injected into the result. + my %tf_map = map { $_ => 1 } Bugzilla::Extension::TrackingFlags::Flag->get_all_names(); + my @tf_selected = grep { exists $tf_map{$_} } @columns; + # mysql has a limit of 61 joins, and we want to avoid massive amounts of joins + # 30 ensures we won't hit the limit, nor generate too many joins + if (scalar @tf_selected > 30) { + foreach my $column (@tf_selected) { + $self->COLUMNS->{$column}->{name} = "'---'"; + } + $self->{tracking_flags} = \@tf_selected; + } + else { + $self->{tracking_flags} = []; + } + # Only add columns which are not already listed. my %list = map { $_ => 1 } @columns; foreach my $column ($self->_extra_columns) { @@ -1960,8 +2024,9 @@ sub _get_column_joins { return $cache->{column_joins} if defined $cache->{column_joins}; my %column_joins = %{ COLUMN_JOINS() }; + # BMO - add search object to hook Bugzilla::Hook::process('buglist_column_joins', - { column_joins => \%column_joins }); + { column_joins => \%column_joins, search => $self }); $cache->{column_joins} = \%column_joins; return $cache->{column_joins}; diff --git a/buglist.cgi b/buglist.cgi index 6ffbea94f..9c7281822 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -546,16 +546,7 @@ if (defined $params->param('columnlist')) { if ($params->param('columnlist') eq "all") { # If the value of the CGI parameter is "all", display all columns, # but remove the redundant "short_desc" column. - # BMO: Skip tracking flag columns when retrieving all columns - # MySQL bombs on greater than 61 joins. - my @non_tf_columns; - foreach my $column (keys %$columns) { - next if $column eq 'short_desc'; - next if ($column =~ /^cf_(blocking|tracking|status)/ - && $columns->{$column}->{name} =~ /^COALESCE/); - push(@non_tf_columns, $column); - } - @displaycolumns = @non_tf_columns; + @displaycolumns = grep($_ ne 'short_desc', keys(%$columns)); } else { @displaycolumns = split(/[ ,]+/, $params->param('columnlist')); diff --git a/extensions/TrackingFlags/Extension.pm b/extensions/TrackingFlags/Extension.pm index bbf1650d5..0432e7d2f 100644 --- a/extensions/TrackingFlags/Extension.pm +++ b/extensions/TrackingFlags/Extension.pm @@ -341,6 +341,10 @@ sub buglist_columns { sub buglist_column_joins { my ($self, $args) = @_; + # if there are elements in the tracking_flags array, then they have been + # removed from the query, so we mustn't generate joins + return if scalar @{ $args->{search}->{tracking_flags} }; + my $column_joins = $args->{'column_joins'}; my @tracking_flags = Bugzilla::Extension::TrackingFlags::Flag->get_all; foreach my $flag (@tracking_flags) { diff --git a/extensions/TrackingFlags/lib/Flag.pm b/extensions/TrackingFlags/lib/Flag.pm index 08886c267..bc422243e 100644 --- a/extensions/TrackingFlags/lib/Flag.pm +++ b/extensions/TrackingFlags/lib/Flag.pm @@ -214,6 +214,17 @@ sub get_all { values %{ $cache->{'tracking_flags'} }; } +# avoids the overhead of pre-loading if just the field names are required +sub get_all_names { + my $self = shift; + my $cache = Bugzilla->request_cache; + if (!exists $cache->{'tracking_flags_names'}) { + $cache->{'tracking_flags_names'} = + Bugzilla->dbh->selectcol_arrayref("SELECT name FROM tracking_flags ORDER BY name"); + } + return @{ $cache->{'tracking_flags_names'} }; +} + sub remove_from_db { my $self = shift; my $dbh = Bugzilla->dbh; -- cgit v1.2.3-24-g4f1b