From 76db56635e4277804982f04b3828a8cd88093963 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Thu, 27 Mar 2008 10:08:07 +0000 Subject: Bug 372795: Implement Bugzilla::Product::preload() to speed up query.cgi when there are many products Patch By Max Kanat-Alexander r=LpSolit, a=mkanat --- Bugzilla/Object.pm | 83 ++++++++++++++++++++++++++--------------------------- Bugzilla/Product.pm | 43 ++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 46 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 23a88855f..d616bb2da 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -129,52 +129,46 @@ sub new_from_list { my $invocant = shift; my $class = ref($invocant) || $invocant; my ($id_list) = @_; - my $dbh = Bugzilla->dbh; - my $columns = join(',', $class->DB_COLUMNS); - my $table = $class->DB_TABLE; - my $order = $class->LIST_ORDER; my $id_field = $class->ID_FIELD; - my $objects; - if (@$id_list) { - my @detainted_ids; - foreach my $id (@$id_list) { - detaint_natural($id) || - ThrowCodeError('param_must_be_numeric', - {function => $class . '::new_from_list'}); - push(@detainted_ids, $id); - } - $objects = $dbh->selectall_arrayref( - "SELECT $columns FROM $table WHERE " - . $dbh->sql_in($id_field, \@detainted_ids) - . "ORDER BY $order", {Slice=>{}}); - } else { - return []; + my @detainted_ids; + foreach my $id (@$id_list) { + detaint_natural($id) || + ThrowCodeError('param_must_be_numeric', + {function => $class . '::new_from_list'}); + push(@detainted_ids, $id); } - - foreach my $object (@$objects) { - bless($object, $class); - } - return $objects; + # We don't do $invocant->match because some classes have + # their own implementation of match which is not compatible + # with this one. However, match() still needs to have the right $invocant + # in order to do $class->DB_TABLE and so on. + return match($invocant, { $id_field => \@detainted_ids }); } # Note: Future extensions to this could be: -# * Accept arrays for an IN clause # * Add a MATCH_JOIN constant so that we can join against # certain other tables for the WHERE criteria. sub match { my ($invocant, $criteria) = @_; my $class = ref($invocant) || $invocant; my $dbh = Bugzilla->dbh; - my $id = $class->ID_FIELD; - my $table = $class->DB_TABLE; return [$class->get_all] if !$criteria; my (@terms, @values); foreach my $field (keys %$criteria) { my $value = $criteria->{$field}; - if ($value eq NOT_NULL) { + if (ref $value eq 'ARRAY') { + # IN () is invalid SQL, and if we have an empty list + # to match against, we're just returning an empty + # array anyhow. + return [] if !scalar @$value; + + my @qmarks = ("?") x @$value; + push(@terms, $dbh->sql_in($field, \@qmarks)); + push(@values, @$value); + } + elsif ($value eq NOT_NULL) { push(@terms, "$field IS NOT NULL"); } elsif ($value eq IS_NULL) { @@ -187,11 +181,25 @@ sub match { } my $where = join(' AND ', @terms); - my $ids = $dbh->selectcol_arrayref( - "SELECT $id FROM $table WHERE $where", undef, @values) - || []; + return $class->_do_list_select($where, \@values); +} + +sub _do_list_select { + my ($class, $where, $values) = @_; + my $table = $class->DB_TABLE; + my $cols = join(',', $class->DB_COLUMNS); + my $order = $class->LIST_ORDER; + + my $sql = "SELECT $cols FROM $table"; + if (defined $where) { + $sql .= " WHERE $where "; + } + $sql .= " ORDER BY $order"; - return $class->new_from_list($ids); + my $dbh = Bugzilla->dbh; + my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @$values); + bless ($_, $class) foreach @$objects; + return $objects } ############################### @@ -349,16 +357,7 @@ sub insert_create_data { sub get_all { my $class = shift; - my $dbh = Bugzilla->dbh; - my $table = $class->DB_TABLE; - my $order = $class->LIST_ORDER; - my $id_field = $class->ID_FIELD; - - my $ids = $dbh->selectcol_arrayref(qq{ - SELECT $id_field FROM $table ORDER BY $order}); - - my $objects = $class->new_from_list($ids); - return @$objects; + return @{$class->_do_list_select()}; } ############################### diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 45c489973..edc621f67 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -51,6 +51,32 @@ use constant DB_COLUMNS => qw( products.defaultmilestone ); +############################### +#### Constructors ##### +############################### + +# This is considerably faster than calling new_from_list three times +# for each product in the list, particularly with hundreds or thousands +# of products. +sub preload { + my ($products) = @_; + my %prods = map { $_->id => $_ } @$products; + my @prod_ids = keys %prods; + return unless @prod_ids; + + my $dbh = Bugzilla->dbh; + foreach my $field (qw(component version milestone)) { + my $classname = "Bugzilla::" . ucfirst($field); + my $objects = $classname->match({ product_id => \@prod_ids }); + + # Now populate the products with this set of objects. + foreach my $obj (@$objects) { + my $product_id = $obj->product_id; + $prods{$product_id}->{"${field}s"} ||= []; + push(@{$prods{$product_id}->{"${field}s"}}, $obj); + } + } +} ############################### #### Methods #### @@ -300,7 +326,7 @@ below. =over -=item C +=item C Description: Returns an array of component objects belonging to the product. @@ -319,7 +345,7 @@ below. Returns: A hash with group id as key and hash containing a Bugzilla::Group object and the properties of group relative to the product. - + =item C =over @@ -356,7 +382,7 @@ groups, in this product. =back -=item C +=item C Description: Returns all valid versions for that product. @@ -364,7 +390,7 @@ groups, in this product. Returns: An array of Bugzilla::Version objects. -=item C +=item C Description: Returns all valid milestones for that product. @@ -415,6 +441,15 @@ groups, in this product. =over +=item C + +When passed an arrayref of C objects, preloads their +L, L, and L, which is much faster +than calling those accessors on every item in the array individually. + +This function is not exported, so must be called like +C. + =item C Description: Checks if the product name was passed in and if is a valid -- cgit v1.2.3-24-g4f1b