Bugzilla – Attachment 194108 Details for
Bug 41950
Make +count embeds sortable by using SQL-level COUNT subqueries
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41950: Make +count embeds sortable via SQL COUNT subqueries
305de37.patch (text/plain), 7.88 KB, created by
Tomás Cohen Arazi (tcohen)
on 2026-02-27 16:51:39 UTC
(
hide
)
Description:
Bug 41950: Make +count embeds sortable via SQL COUNT subqueries
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2026-02-27 16:51:39 UTC
Size:
7.88 KB
patch
obsolete
>From 305de37e932a810edabdc763bd89eb25668c4b90 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >Date: Fri, 27 Feb 2026 12:31:30 -0300 >Subject: [PATCH] Bug 41950: Make +count embeds sortable via SQL COUNT > subqueries >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Currently, +count embeds (e.g. claims+count) are computed in Perl via >$object->$relation->count during to_api serialization. This means they >cannot be used in _order_by and each count triggers a separate SQL query >per row (N+1). > >This patch makes dbic_merge_prefetch detect +count embeds and, when a >matching DBIC relationship exists, inject a correlated COUNT subquery >via +select/+as instead of adding a prefetch. > >The order_by entries referencing count aliases are fixed up to strip the >me. prefix, since +as aliases are virtual columns not bound to a table. > >In to_api, the is_count handler now checks for the pre-computed column >via get_column before falling back to the Perl-level ->count call. This >preserves backward compatibility for Koha-level methods that don't have >a corresponding DBIC relationship. > >To test: >1. Run: > $ prove t/Koha/REST/Plugin/Query.t > => SUCCESS >2. Run: > $ prove t/db_dependent/Koha/Object.t > => SUCCESS >3. Run: > $ prove t/db_dependent/Koha/REST/Plugin/Objects.t > => SUCCESS >4. Run: > $ prove -r t/db_dependent/api/v1 > => SUCCESS > >Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> >--- > Koha/Object.pm | 13 +++++-- > Koha/REST/Plugin/Query.pm | 81 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 91 insertions(+), 3 deletions(-) > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index 55c7e489a0..9f6fd1062d 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -203,7 +203,7 @@ sub store { > value => $value, > property => $property =~ /(\w+\.\w+)$/ > ? $1 >- : $property, # results in table.column without quotes or backtics >+ : $property, # results in table.column without quotes or backticks > ); > } elsif ( $_->{msg} =~ /Data truncated for column \W?(?<property>\w+)/ ) > { # The optional \W in the regex might be a quote or backtick >@@ -213,7 +213,7 @@ sub store { > type => 'enum', > property => $property =~ /(\w+\.\w+)$/ > ? $1 >- : $property, # results in table.column without quotes or backtics >+ : $property, # results in table.column without quotes or backticks > value => $self->$property, > ) if $type eq 'enum'; > } >@@ -680,7 +680,14 @@ sub to_api { > { > > my $relation = $+{relation}; >- $json_object->{$embed} = $self->$relation->count; >+ >+ # If the query was built with a SQL-level COUNT subquery (via +select/+as >+ # in _merge_count_select), the count is already available as a virtual column >+ # on the DBIC row. Use it to avoid an N+1 query per row. >+ # Falls back to the Koha relation ->count call for cases where no DBIC >+ # relationship exists (e.g. a Koha-only method like the old claims_count). >+ my $count = eval { $self->_result->get_column($embed) }; >+ $json_object->{$embed} = defined $count ? $count : $self->$relation->count; > } else { > my $curr = $embed; > my $next = $embeds->{$curr}->{children}; >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index 5544d02ead..cb19cab74c 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -148,7 +148,22 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in > return unless defined $embed; > > my @prefetches; >+ my $count_aliases = {}; >+ > foreach my $key ( sort keys( %{$embed} ) ) { >+ >+ # Handle +count embeds with SQL-level subquery >+ if ( $embed->{$key}->{is_count} ) { >+ my $added = _merge_count_select( $key, $attributes, $result_set ); >+ if ($added) { >+ $count_aliases->{$key} = 1; >+ next; >+ } >+ >+ # No DBIC relationship found; fall through so to_api >+ # can use the Koha method ->count fallback >+ } >+ > my $parsed = _parse_prefetch( $key, $embed, $result_set ); > push @prefetches, $parsed if defined $parsed; > } >@@ -156,6 +171,25 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in > if ( scalar(@prefetches) ) { > $attributes->{prefetch} = \@prefetches; > } >+ >+ # Fix order_by entries referencing count aliases (me.X_count => X_count). >+ # dbic_merge_sorting (which runs before this) translates _order_by=-me.claims_count >+ # into { -desc => 'me.claims_count' }. But +as aliases are not real table columns, >+ # so DBIC needs them without the 'me.' prefix to match the +select subquery. >+ if ( %{$count_aliases} && $attributes->{order_by} ) { >+ for my $i ( 0 .. $#{ $attributes->{order_by} } ) { >+ my $atom = $attributes->{order_by}[$i]; >+ if ( ref($atom) eq 'HASH' ) { >+ for my $dir ( keys %{$atom} ) { >+ ( my $col = $atom->{$dir} ) =~ s/^me\.//; >+ $atom->{$dir} = $col if $count_aliases->{$col}; >+ } >+ } elsif ( !ref($atom) ) { >+ ( my $col = $atom ) =~ s/^me\.//; >+ $attributes->{order_by}[$i] = $col if $count_aliases->{$col}; >+ } >+ } >+ } > } > ); > >@@ -449,6 +483,53 @@ sub _merge_embed { > } > } > >+=head3 _merge_count_select >+ >+ my $added = _merge_count_select( $key, $attributes, $result_set ); >+ >+Given a I<$key> like C<claims_count> (from a C<claims+count> embed), strips the C<_count> >+suffix to derive the DBIC relationship name, then uses the relationship metadata to build >+a correlated COUNT subquery added via C<+select>/C<+as>. >+ >+This makes the count available as a virtual column in the SQL result, enabling both >+server-side sorting (ORDER BY) and an efficient single-query count instead of N+1. >+ >+Returns 1 if the subquery was added, or undef if the relationship was not found in the >+C<prefetch_whitelist> or has no DBIC relationship info. In the latter case, the caller >+should fall through to the Koha-level C<< $object->$relation->count >> path in C<to_api>. >+ >+Handles both C<Koha::Objects> (plural, via C<_resultset>) and C<Koha::Object> (singular, >+via C<_result>) for accessing the DBIC result source. >+ >+=cut >+ >+sub _merge_count_select { >+ my ( $key, $attributes, $result_set ) = @_; >+ >+ ( my $rel = $key ) =~ s/_count$//; >+ return unless exists $result_set->prefetch_whitelist->{$rel}; >+ >+ my $source = >+ $result_set->can('_resultset') >+ ? $result_set->_resultset->result_source >+ : $result_set->_result->result_source; >+ my $rel_info = $source->relationship_info($rel); >+ return unless $rel_info; >+ >+ my $rel_table = $source->related_source($rel)->name; >+ my $cond = $rel_info->{cond}; >+ my ($fk_foreign) = keys %{$cond}; >+ my $fk_self = $cond->{$fk_foreign}; >+ $fk_foreign =~ s/^foreign\.//; >+ $fk_self =~ s/^self\.//; >+ >+ push @{ $attributes->{'+select'} }, >+ \"(SELECT COUNT(*) FROM $rel_table WHERE $rel_table.$fk_foreign = me.$fk_self)"; >+ push @{ $attributes->{'+as'} }, $key; >+ >+ return 1; >+} >+ > sub _parse_prefetch { > my ( $key, $embed, $result_set ) = @_; > >-- >2.50.1 (Apple Git-155) >
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 41950
:
194107
|
194108
|
194191
|
194192