From 82687e8be814421f5450dc9912960ae995e3187b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 10 Jul 2023 17:03:34 +0000 Subject: [PATCH] Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-data.sh) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim Signed-off-by: Laura Escamilla --- Koha/Illbackend.pm | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Koha/Illbackend.pm b/Koha/Illbackend.pm index 8a6be8a7a6..cb27e21154 100644 --- a/Koha/Illbackend.pm +++ b/Koha/Illbackend.pm @@ -48,16 +48,19 @@ Return a list of existing ILL statuses sub existing_statuses { my ( $self, $backend_id ) = @_; - #FIXME: Currently fetching all requests, it'd be great if we could fetch distinct(status). - # Even doing it with distinct status, we need the ILL request object, so that strings_map works and - # the ILL request returns the correct status and info respective to its backend. + # NOTE: This query fetches all distinct status's found in the database for this backend. + # We need the 'status' field for obvious reasons, the 'backend' field is required to not + # throw 'Koha::Exceptions::Ill::InvalidBackendId' when we're converting to a Koha object. + # Finally, to get around 'ONLY_FULL_GROUP_BY', we have to be explicit about which + # 'request_id' we want to return, hense the 'MAX' call. my $ill_requests = Koha::Illrequests->search( - {backend => $backend_id}, - # { - # columns => [ qw/status/ ], - # group_by => [ qw/status/ ], - # } - ); + { backend => $backend_id }, + { + select => [ 'status', \'MAX(illrequest_id)', 'backend' ], + as => [qw/ status illrequest_id backend /], + group_by => [qw/status backend/], + } + ); my @data; while (my $request = $ill_requests->next) { @@ -74,10 +77,6 @@ sub existing_statuses { } } - # Remove duplicate statuses - my %seen; - @data = grep { my $e = $_; my $key = join '___', map { $e->{$_}; } sort keys %$_;!$seen{$key}++ } @data; - return \@data; } -- 2.30.2