Attachment 153962 Details for Bug 34223 – Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend

[patch] Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend

Bug-34223-Work-around-ONLYFULLGROUPBY-in-Illbacken.patch (text/plain), 2.95 KB, created by Martin Renvoize (ashimema) on 2023-07-27 11:44:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-07-27 11:44:28 UTC

Size: 2.95 KB

patch

obsolete

>From 22d8f67df87fc0eb9a233f33f72f337557414285 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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 <pedro.amorim@ptfs-europe.com>
>---
> Koha/Illbackend.pm | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
>diff --git a/Koha/Illbackend.pm b/Koha/Illbackend.pm
>index 8a6be8a7a6..017619febb 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.41.0

Actions: View | Diff | Splinter Review

Attachments on bug 34223: 153289 | 153953 | 153956 | 153957 | 153961 | 153962 | 153963 | 153968 | 153973 | 153978 | 153979 | 153980 | 153981 | 153989 | 153990 | 154026 | 154027 | 154028 | 154029 | 154030 | 154031 | 154032 | 154033 | 154068 | 154069 | 154070 | 154071 | 155280 | 155493