Bugzilla – Attachment 169189 Details for
Bug 37389
REST API queries joining on extended_attributes may cause severe performance issues
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37389:
Bug-37389.patch (text/plain), 9.08 KB, created by
Martin Renvoize
on 2024-07-19 09:54:28 UTC
(
hide
)
Description:
Bug 37389:
Filename:
MIME Type:
Creator:
Martin Renvoize
Created:
2024-07-19 09:54:28 UTC
Size:
9.08 KB
patch
obsolete
>From f1fdb1edfd33d265d655f995dfb54cd86705a005 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Thu, 18 Jul 2024 13:04:22 +0100 >Subject: [PATCH] Bug 37389: > >This patch adds a number of methods that work together to result the >problem as a whole. > >We add a builder method to dynamically create DBIx::Class relations when >required for the Koha::ILL::Requests resultset. > >We update Koha::REST::Plugin::Query to call the aforementioned builder >method so we can subsequently use these new relations to create a >performance join condition. (This also required passing the ResultSet >into the dbic_extended_attributes_join helper from >Koha::REST::Plugin::Objects) > >We that add an AUTOLOAD into Koha::ILL::Request to allow use of the >new dynamically added DBIx::Class relations in the above as >extended_attributes_$type from the corresponding Koha::Object. > >Finally, we reworked the query builder code to ensure we're using the >new relations added when using the API. >--- > Koha/ILL/Request.pm | 36 ++++++++++++++++++++++++ > Koha/ILL/Requests.pm | 40 ++++++++++++++++++++++++++ > Koha/REST/Plugin/Objects.pm | 3 +- > Koha/REST/Plugin/Query.pm | 56 ++++++++++++++++++------------------- > 4 files changed, 106 insertions(+), 29 deletions(-) > >diff --git a/Koha/ILL/Request.pm b/Koha/ILL/Request.pm >index e7347e5ee32..d8b669fe95a 100644 >--- a/Koha/ILL/Request.pm >+++ b/Koha/ILL/Request.pm >@@ -2220,6 +2220,42 @@ sub get_staff_table_actions { > return $ill_table_actions; > } > >+=head3 AUTOLOAD >+ >+=cut >+ >+our $AUTOLOAD; >+ >+sub AUTOLOAD { >+ my ($self) = @_; >+ >+ my $name = $AUTOLOAD; >+ $name =~ s/.*:://; # Remove package name >+ >+ if ( $name =~ /^extended_attributes_(\w+)$/ ) { >+ my $type = $1; >+ >+ # Define the method dynamically >+ no strict 'refs'; >+ *{$AUTOLOAD} = sub { >+ my ($self) = @_; >+ my $relation = 'extended_attributes_' . $type; >+ my $rs = $self->_result->$relation; >+ return Koha::ILL::Request::Attributes->_new_from_dbic($rs)->search; >+ }; >+ >+ # Call the newly defined method >+ return $self->$name(); >+ } >+ my $wt = 'SUPER::'.$name; >+ return $self->$wt; >+} >+ >+sub get_column(){ >+ my ($self, $column_name) = @_; >+ return $self->_result->get_column($column_name); >+} >+ > =head3 _type > > =cut >diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm >index 468bd34d825..46fbc9d9ce3 100644 >--- a/Koha/ILL/Requests.pm >+++ b/Koha/ILL/Requests.pm >@@ -108,6 +108,46 @@ sub search_incomplete { > > =head2 Internal methods > >+ >+=head3 _build_extended_attributes_relations >+ >+Method to dynamically add has_many relations for each ILLRequestAttribute type stored in the ILLRequestAttributes table. >+ >+Used in the API to allow for advanced joins. >+ >+Returns a list of relation accessor names. >+ >+=cut >+ >+sub _build_extended_attributes_relations { >+ my ( $self, $types ) = @_; >+ >+ my $result_source = $self->_resultset->result_source; >+ for my $type ( @{$types} ) { >+ $result_source->add_relationship( >+ "extended_attributes_$type", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', $type }, >+ }; >+ }, >+ { >+ accessor => 'multi', >+ join_type => 'LEFT', >+ cascade_copy => 0, >+ cascade_delete => 0, >+ is_depends_on => 0 >+ }, >+ ); >+ >+ } >+ return map { 'extended_attributes_' . $_ } @{$types}; >+} >+ > =head3 _type > > =cut >diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm >index 7f06582b4dd..d14d8c99a4a 100644 >--- a/Koha/REST/Plugin/Objects.pm >+++ b/Koha/REST/Plugin/Objects.pm >@@ -291,7 +291,8 @@ controller, and thus shouldn't be called twice in it. > $c->dbic_extended_attributes_join( > { > attributes => $attributes, >- filtered_params => $filtered_params >+ filtered_params => $filtered_params, >+ result_set => $result_set > } > ); > >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index 53decc94bcf..fd9dcf04822 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -173,16 +173,18 @@ Generates the DBIC join attribute based on extended_attributes query entries, an > > my $attributes = $args->{attributes}; > my $filtered_params = $args->{filtered_params}; >+ my $result_set = $args->{result_set}; > > if ( reftype( $attributes->{prefetch} ) > && reftype( $attributes->{prefetch} ) eq 'ARRAY' > && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) ) > { >- my $ea_entries = $self->_get_extended_attributes_entries( $filtered_params, 0 ); >- while ( $ea_entries > 0 ) { >- push( @{ $attributes->{join} }, 'extended_attributes' ); >- $ea_entries--; >- } >+ my @array = $self->_get_extended_attributes_entries( $filtered_params, 0 ); >+ >+ # Calling our private method to build the extended attributes relations >+ my @joins = $result_set->_build_extended_attributes_relations(\@array); >+ push @{ $attributes->{join} }, @joins; >+ > } > } > ); >@@ -557,36 +559,35 @@ Example: Returns 2 if given a $filtered_params containing the below: > =cut > > sub _get_extended_attributes_entries { >- my ( $self, $params, $extended_attributes_entries ) = @_; >+ my ( $self, $params, $extended_attributes_entries, @array ) = @_; > > if ( reftype($params) && reftype($params) eq 'HASH' ) { > > # rewrite additional_field_values table query params >- $extended_attributes_entries = >- _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' ) >+ @array = >+ _rewrite_related_metadata_query( $params, 'field_id', 'value', @array ) > if $params->{'extended_attributes.field_id'}; > > # rewrite borrower_attributes table query params >- $extended_attributes_entries = >- _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'code', 'attribute' ) >+ @array = >+ _rewrite_related_metadata_query( $params, 'code', 'attribute', @array ) > if $params->{'extended_attributes.code'}; > > # rewrite illrequestattributes table query params >- $extended_attributes_entries = >- _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'type', 'value' ) >+ @array = >+ _rewrite_related_metadata_query( $params, 'type', 'value', @array ) > if $params->{'extended_attributes.type'}; > > foreach my $key ( keys %{$params} ) { >- return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries ); >+ return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries, @array ); > } > } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) { > foreach my $ea_instance (@$params) { >- $extended_attributes_entries = >- +$self->_get_extended_attributes_entries( $ea_instance, $extended_attributes_entries ); >+ @array = $self->_get_extended_attributes_entries( $ea_instance, $extended_attributes_entries, @array ); > } >- return $extended_attributes_entries; >+ return @array; > } else { >- return $extended_attributes_entries; >+ return @array; > } > } > >@@ -625,20 +626,19 @@ It'll be rewritten as: > =cut > > sub _rewrite_related_metadata_query { >- my ( $params, $extended_attributes_entries, $key, $value ) = @_; >+ my ( $params, $key, $value, @array ) = @_; > >- $extended_attributes_entries++; >- if ( $extended_attributes_entries > 1 ) { >- my $old_key_value = delete $params->{ 'extended_attributes.' . $key }; >- my $new_key_value = "extended_attributes_$extended_attributes_entries" . "." . $key; >- $params->{$new_key_value} = $old_key_value; >+ my $old_key_value = delete $params->{ 'extended_attributes.' . $key }; >+ my $new_key_value = "extended_attributes_$old_key_value" . "." . $key; >+ $params->{$new_key_value} = $old_key_value; > >- my $old_value_value = delete $params->{ 'extended_attributes.' . $value }; >- my $new_value_value = "extended_attributes_$extended_attributes_entries" . "." . $value; >- $params->{$new_value_value} = $old_value_value; >- } >+ my $old_value_value = delete $params->{ 'extended_attributes.' . $value }; >+ my $new_value_value = "extended_attributes_$old_key_value" . "." . $value; >+ $params->{$new_value_value} = $old_value_value; >+ >+ push @array, $old_key_value; > >- return $extended_attributes_entries; >+ return @array; > } > > 1; >-- >2.45.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37389
:
169130
|
169133
|
169140
|
169149
|
169159
|
169178
|
169185
|
169189
|
169191
|
169192
|
169234
|
169243
|
169244
|
169245
|
169246
|
169317
|
169318
|
169569
|
169570
|
169583
|
169584
|
169585
|
169586
|
169850
|
169851
|
169944
|
169945
|
169946
|
169954
|
169955
|
169956