Bugzilla – Attachment 169570 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: ExtendedAttributes mixin
Bug-37389-ExtendedAttributes-mixin.patch (text/plain), 18.36 KB, created by
Pedro Amorim
on 2024-07-25 15:39:14 UTC
(
hide
)
Description:
Bug 37389: ExtendedAttributes mixin
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2024-07-25 15:39:14 UTC
Size:
18.36 KB
patch
obsolete
>From 9b87bf087fffbfa2cba65b16c64b0f82278885d4 Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@ptfs-europe.com> >Date: Thu, 25 Jul 2024 15:28:37 +0000 >Subject: [PATCH] Bug 37389: ExtendedAttributes mixin > >This is what we're doing here: > >- Creating a new mixin called ExtendedAttributes.pm >- Applying this Mixin to Patrons and ILL::Requests (we don't apply it to AdditionalFields.pm here yet because no AdditionalFields supporting classes have the extended_attributes accessor yet, I'll tackle this when rebasing 35287) > >- The aforementioned mixin does the following: >-- Generates dynamic accessors for extended_attributes e.g. if there is a borrower attribute with code 'height', the 'extended_attributes_height' is generated dynamically when a search with 'prefetch'=>'extended_attributes' is performed. >-- Rewrites the 'join' entries in the query to have the alias as above. >-- Rewrites the WHERE conditions to match the above ruleset. > >Example: > >A DBIX search query as follow: > >[ > { > '-and' => [ > [ > { > 'extended_attributes.attribute' => { 'like' => 'abc%' }, > 'extended_attributes.code' => 'CODE_1' > } > ], > [ > { > 'extended_attributes.code' => 'CODE_2', > 'extended_attributes.attribute' => { 'like' => '123%' } > } > ] > ] > } > ] > >Results in the following SQL: > > SELECT > `me`.`borrowernumber` > FROM > `borrowers` `me` > LEFT JOIN `borrower_attributes` `extended_attributes_CODE_1` ON ( > `extended_attributes_CODE_1`.`borrowernumber` = `me`.`borrowernumber` > AND `extended_attributes_CODE_1`.`code` = ? > ) > LEFT JOIN `borrower_attributes` `extended_attributes_CODE_2` ON ( > `extended_attributes_CODE_2`.`borrowernumber` = `me`.`borrowernumber` > AND `extended_attributes_CODE_2`.`code` = ? > ) > WHERE > ( > ( > ( > `extended_attributes_CODE_1`.`attribute` LIKE ? > AND `extended_attributes_CODE_1`.`code` = ? > ) > AND ( > `extended_attributes_CODE_2`.`attribute` LIKE ? > AND `extended_attributes_CODE_2`.`code` = ? > ) > ) > ) > >What fixes the performance issue that originated this work is the 'AND `extended_attributes_CODE_1`.`code` = ?' that was missing on the LEFT JOIN. > >All of the above is explained using Borrowers and Borrower attributes, but it all also applies to ILL::Requests and ILL::Request::Attributes. > >Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/ILL/Requests.pm | 16 +- > Koha/Objects/Mixin/ExtendedAttributes.pm | 234 +++++++++++++++++++++++ > Koha/Patrons.pm | 15 +- > Koha/REST/Plugin/Objects.pm | 8 - > Koha/REST/Plugin/Query.pm | 153 --------------- > 5 files changed, 263 insertions(+), 163 deletions(-) > create mode 100644 Koha/Objects/Mixin/ExtendedAttributes.pm > >diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm >index 468bd34d82..39ca48d3e9 100644 >--- a/Koha/ILL/Requests.pm >+++ b/Koha/ILL/Requests.pm >@@ -22,8 +22,9 @@ use Modern::Perl; > use Koha::Database; > use Koha::ILL::Request; > use Koha::ILL::Request::Config; >+use Koha::Objects::Mixin::ExtendedAttributes; > >-use base qw(Koha::Objects); >+use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects); > > =head1 NAME > >@@ -106,6 +107,19 @@ sub search_incomplete { > } ); > } > >+=head3 extended_attributes_config >+ >+=cut >+ >+sub extended_attributes_config { >+ my ( $self ) = @_; >+ return { >+ 'id_field' => { 'foreign' => 'illrequest_id', 'self' => 'illrequest_id' }, >+ 'key_field' => 'type', >+ 'schema_class' => 'Koha::Schema::Result::Illrequestattribute', >+ }; >+} >+ > =head2 Internal methods > > =head3 _type >diff --git a/Koha/Objects/Mixin/ExtendedAttributes.pm b/Koha/Objects/Mixin/ExtendedAttributes.pm >new file mode 100644 >index 0000000000..386d657535 >--- /dev/null >+++ b/Koha/Objects/Mixin/ExtendedAttributes.pm >@@ -0,0 +1,234 @@ >+package Koha::Objects::Mixin::ExtendedAttributes; >+ >+# Copyright 2024 PTFS Europe Ltd >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+use Scalar::Util qw( reftype ); >+ >+=head1 NAME >+ >+Koha::Objects::Mixin::ExtendedAttributes >+ >+=head2 Class methods >+ >+ >+=head3 search >+ >+ Overwrites the search method to include extended attributes rewrite and dynamic relation accessors >+ >+=cut >+ >+sub search { >+ my ( $self, $params, $attributes ) = @_; >+ >+ my $class = ref($self) ? ref($self) : $self; >+ >+ $self->handle_query_extended_attributes( >+ { >+ attributes => $attributes, >+ filtered_params => $params, >+ } >+ ); >+ >+ my $rs = $self->_resultset()->search( $params, $attributes ); >+ return $class->_new_from_dbic($rs); >+} >+ >+=head3 handle_query_extended_attributes >+ >+ Checks for the presence of extended_attributes in a query >+ If present, it builds the dynamic extended attributes relations and rewrites the query to include the extended_attributes relation >+ >+=cut >+ >+sub handle_query_extended_attributes { >+ my ( $self, $args ) = @_; >+ >+ my $attributes = $args->{attributes}; >+ my $filtered_params = $args->{filtered_params}; >+ >+ if ( reftype( $attributes->{prefetch} ) >+ && reftype( $attributes->{prefetch} ) eq 'ARRAY' >+ && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) ) >+ { >+ >+ my @array = $self->_get_extended_attributes_entries($filtered_params); >+ >+ # Calling our private method to build the extended attributes relations >+ my @joins = $self->_build_extended_attributes_relations( \@array ); >+ push @{ $attributes->{join} }, @joins; >+ >+ } >+} >+ >+=head3 _get_extended_attributes_entries >+ >+ $self->_get_extended_attributes_entries( $filtered_params, 0 ) >+ >+Recursive function that returns the rewritten extended_attributes query entries. >+ >+Given: >+[ >+ '-and', >+ [ >+ { >+ 'extended_attributes.code' => 'CODE_1', >+ 'extended_attributes.attribute' => { 'like' => '%Bar%' } >+ }, >+ { >+ 'extended_attributes.attribute' => { 'like' => '%Bar%' }, >+ 'extended_attributes.code' => 'CODE_2' >+ } >+ ] >+]; >+ >+Returns : >+ >+[ >+ 'CODE_1', >+ 'CODE_2' >+] >+ >+=cut >+ >+sub _get_extended_attributes_entries { >+ my ( $self, $params, @array ) = @_; >+ >+ if ( reftype($params) && reftype($params) eq 'HASH' ) { >+ >+ # rewrite additional_field_values table query params >+ @array = _rewrite_related_metadata_query( $params, 'field_id', 'value', @array ) >+ if $params->{'extended_attributes.field_id'}; >+ >+ # rewrite borrower_attributes table query params >+ @array = _rewrite_related_metadata_query( $params, 'code', 'attribute', @array ) >+ if $params->{'extended_attributes.code'}; >+ >+ # rewrite illrequestattributes table query params >+ @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}, @array ); >+ } >+ } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) { >+ foreach my $ea_instance (@$params) { >+ @array = $self->_get_extended_attributes_entries( $ea_instance, @array ); >+ } >+ return @array; >+ } else { >+ return @array; >+ } >+} >+ >+=head3 _rewrite_related_metadata_query >+ >+ $extended_attributes_entries = >+ _rewrite_related_metadata_query( $params, 'field_id', 'value', @array ) >+ >+Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join >+Take the below example (patrons): >+ [ >+ { >+ "extended_attributes.attribute":{"like":"%123%"}, >+ "extended_attributes.code":"CODE_1" >+ } >+ ], >+ [ >+ { >+ "extended_attributes.attribute":{"like":"%abc%" }, >+ "extended_attributes.code":"CODE_2" >+ } >+ ] >+ >+It'll be rewritten as: >+ [ >+ { >+ 'extended_attributes_CODE_1.attribute' => { 'like' => '%123%' }, >+ 'extended_attributes_CODE_1.code' => 'CODE_1' >+ } >+ ], >+ [ >+ { >+ 'extended_attributes_CODE_2.attribute' => { 'like' => '%abc%' }, >+ 'extended_attributes_CODE_2.code' => 'CODE_2' >+ } >+ ] >+ >+=cut >+ >+sub _rewrite_related_metadata_query { >+ my ( $params, $key, $value, @array ) = @_; >+ >+ if ( ref \$params->{ 'extended_attributes.' . $key } eq 'SCALAR' ) { >+ 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_$old_key_value" . "." . $value; >+ $params->{$new_value_value} = $old_value_value; >+ push @array, $old_key_value; >+ } >+ >+ return @array; >+} >+ >+=head3 _build_extended_attributes_relations >+ >+ Method to dynamically add has_many relations for Koha classes that support extended_attributes. >+ >+ Returns a list of relation accessor names. >+ >+=cut >+ >+sub _build_extended_attributes_relations { >+ my ( $self, $types ) = @_; >+ >+ return if( !grep ( /extended_attributes/, keys %{$self->_resultset->result_source->_relationships} ) ); >+ >+ my $ea_config = $self->extended_attributes_config; >+ >+ my $result_source = $self->_resultset->result_source; >+ for my $type ( @{$types} ) { >+ $result_source->add_relationship( >+ "extended_attributes_$type", >+ "$ea_config->{schema_class}", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.$ea_config->{id_field}->{foreign}" => >+ { -ident => "$args->{self_alias}.$ea_config->{id_field}->{self}" }, >+ "$args->{foreign_alias}.$ea_config->{key_field}" => { '=', $type }, >+ }; >+ }, >+ { >+ accessor => 'multi', >+ join_type => 'LEFT', >+ cascade_copy => 0, >+ cascade_delete => 0, >+ is_depends_on => 0 >+ }, >+ ); >+ >+ } >+ return map { 'extended_attributes_' . $_ } @{$types}; >+} >+ >+1; >\ No newline at end of file >diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm >index eb89573312..813d741c2a 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -29,7 +29,7 @@ use Koha::Patron; > use Koha::Exceptions::Patron; > use Koha::Patron::Categories; > >-use base qw(Koha::Objects); >+use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects); > > =head1 NAME > >@@ -566,6 +566,19 @@ sub filter_by_have_permission { > ); > } > >+=head3 extended_attributes_config >+ >+=cut >+ >+sub extended_attributes_config { >+ my ( $self ) = @_; >+ return { >+ 'id_field' => { 'foreign' => 'borrowernumber', 'self' => 'borrowernumber' }, >+ 'key_field' => 'code', >+ 'schema_class' => 'Koha::Schema::Result::BorrowerAttribute', >+ }; >+} >+ > =head3 _type > > =cut >diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm >index 7f06582b4d..32dab06d4b 100644 >--- a/Koha/REST/Plugin/Objects.pm >+++ b/Koha/REST/Plugin/Objects.pm >@@ -287,14 +287,6 @@ controller, and thus shouldn't be called twice in it. > $c->stash('koha.pagination.base_total' => $result_set->count); > $c->stash('koha.pagination.query_params' => $args); > >- # Check and handle related metadata table joins >- $c->dbic_extended_attributes_join( >- { >- attributes => $attributes, >- filtered_params => $filtered_params >- } >- ); >- > # Generate the resultset > my $objects_rs = $result_set->search( $filtered_params, $attributes ); > # Stash the page total if requires, total otherwise >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index 53decc94bc..4fa3709f7c 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -159,34 +159,6 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in > } > ); > >-=head3 dbic_extended_attributes_join >- >- $attributes = $c->dbic_extended_attributes_join({ attributes => $attributes, result_set => $result_set }); >- >-Generates the DBIC join attribute based on extended_attributes query entries, and merges into I<$attributes>. >- >-=cut >- >- $app->helper( >- 'dbic_extended_attributes_join' => sub { >- my ( $c, $args ) = @_; >- >- my $attributes = $args->{attributes}; >- my $filtered_params = $args->{filtered_params}; >- >- 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--; >- } >- } >- } >- ); >- > =head3 _build_query_params_from_api > > my $params = _build_query_params_from_api( $filtered_params, $reserved_params ); >@@ -516,129 +488,4 @@ sub _parse_dbic_query { > } > } > >-=head3 _get_extended_attributes_entries >- >- $self->_get_extended_attributes_entries( $filtered_params, 0 ) >- >-Recursive function that returns the number of extended_attributes entries present in a query. >-Example: Returns 2 if given a $filtered_params containing the below: >- >-{ >- "-and":[ >- [ >- { >- "extended_attributes.value":{ >- "like":"abc%" >- }, >- "extended_attributes.code":[ >- [ >- "arbitrary_attr_code", >- "another_attr_code" >- ] >- ] >- } >- ], >- [ >- { >- "extended_attributes.value":{ >- "like":"123%" >- }, >- "extended_attributes.code":[ >- [ >- "arbitrary_attr_code", >- "another_attr_code" >- ] >- ] >- } >- ] >- ] >-} >- >-=cut >- >-sub _get_extended_attributes_entries { >- my ( $self, $params, $extended_attributes_entries ) = @_; >- >- 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' ) >- 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' ) >- if $params->{'extended_attributes.code'}; >- >- # rewrite illrequestattributes table query params >- $extended_attributes_entries = >- _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'type', 'value' ) >- if $params->{'extended_attributes.type'}; >- >- foreach my $key ( keys %{$params} ) { >- return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries ); >- } >- } 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 ); >- } >- return $extended_attributes_entries; >- } else { >- return $extended_attributes_entries; >- } >-} >- >-=head3 _rewrite_related_metadata_query >- >- $extended_attributes_entries = >- _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' ) >- >-Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join >-Take the below example (patrons), assuming it is the third instance of an extended_attributes query: >-{ >- "extended_attributes.value":{ >- "like":"123%" >- }, >- "extended_attributes.code":[ >- [ >- "arbitrary_attr_code", >- "another_attr_code" >- ] >- ] >-} >- >-It'll be rewritten as: >-{ >- "extended_attributes_3.value":{ >- "like":"123%" >- }, >- "extended_attributes_3.code":[ >- [ >- "arbitrary_attr_code", >- "another_attr_code" >- ] >- ] >-} >- >-=cut >- >-sub _rewrite_related_metadata_query { >- my ( $params, $extended_attributes_entries, $key, $value ) = @_; >- >- $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_value_value = delete $params->{ 'extended_attributes.' . $value }; >- my $new_value_value = "extended_attributes_$extended_attributes_entries" . "." . $value; >- $params->{$new_value_value} = $old_value_value; >- } >- >- return $extended_attributes_entries; >-} >- > 1; >-- >2.39.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