Bugzilla – Attachment 194339 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
84bc671.patch (text/plain), 12.06 KB, created by
Tomás Cohen Arazi (tcohen)
on 2026-03-02 12:31:56 UTC
(
hide
)
Description:
Bug 41950: Make +count embeds sortable via SQL COUNT subqueries
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2026-03-02 12:31:56 UTC
Size:
12.06 KB
patch
obsolete
>From 84bc671fd328bc11305b3a9c5e6bf9f78c2f35ad 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/Exceptions.pm | 8 ++ > Koha/REST/Plugin/Query.pm | 148 ++++++++++++++++++++++++++++++++- > 3 files changed, 163 insertions(+), 6 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/Exceptions.pm b/Koha/REST/Plugin/Exceptions.pm >index 555633bb10..3ad1450176 100644 >--- a/Koha/REST/Plugin/Exceptions.pm >+++ b/Koha/REST/Plugin/Exceptions.pm >@@ -73,6 +73,14 @@ sub register { > error_code => 'invalid_query', > } > ); >+ } elsif ( blessed $exception && ref($exception) eq 'Koha::Exceptions::BadParameter' ) { >+ return $c->render( >+ status => 400, >+ json => { >+ error => "$exception", >+ error_code => 'bad_parameter', >+ } >+ ); > } elsif ( blessed $exception > && ref($exception) eq 'Koha::Exceptions::REST::Public::Authentication::Required' ) > { >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index 5544d02ead..417a006321 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -148,14 +148,68 @@ 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} ) ) { >- my $parsed = _parse_prefetch( $key, $embed, $result_set ); >+ >+ # Handle +count embeds with SQL-level subquery >+ if ( $embed->{$key}->{is_count} ) { >+ my $subquery = _merge_count_select( $key, $attributes, $result_set ); >+ if ($subquery) { >+ $count_aliases->{$key} = $subquery; >+ 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, $attributes, undef, $count_aliases ); > push @prefetches, $parsed if defined $parsed; > } > > 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 ( $attributes->{order_by} ) { >+ >+ # Collect is_count embeds that lack a DBIC relationship (no SQL-level count) >+ my $unsortable_counts = {}; >+ for my $key ( keys %{$embed} ) { >+ $unsortable_counts->{$key} = 1 >+ if $embed->{$key}->{is_count} && !$count_aliases->{$key}; >+ } >+ >+ 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\.//; >+ >+ # Reject sorting on +count embeds without a DBIC relationship >+ Koha::Exceptions::BadParameter->throw( >+ "Cannot sort on $col: no matching database relationship") >+ if $unsortable_counts->{$col}; >+ >+ # Replace with the literal subquery so DBIC puts it directly >+ # in the ORDER BY clause (DBIC +as is Perl-side only, not a SQL alias) >+ $atom->{$dir} = $count_aliases->{$col} if $count_aliases->{$col}; >+ } >+ } elsif ( !ref($atom) ) { >+ ( my $col = $atom ) =~ s/^me\.//; >+ >+ Koha::Exceptions::BadParameter->throw("Cannot sort on $col: no matching database relationship") >+ if $unsortable_counts->{$col}; >+ >+ $attributes->{order_by}[$i] = $count_aliases->{$col} if $count_aliases->{$col}; >+ } >+ } >+ } > } > ); > >@@ -449,19 +503,107 @@ 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 the subquery scalar reference if added (used by the caller to substitute >+into ORDER BY clauses), 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 $subquery = _build_count_subquery( $rel, $result_set, 'me' ); >+ return unless $subquery; >+ >+ push @{ $attributes->{'+select'} }, $subquery; >+ push @{ $attributes->{'+as'} }, $key; >+ >+ return $subquery; >+} >+ >+=head3 _build_count_subquery >+ >+ my $subquery = _build_count_subquery( $rel, $result_set, $parent_alias ); >+ >+Builds a correlated COUNT subquery scalar ref for the given DBIC relationship. >+I<$parent_alias> is the SQL alias of the parent table (e.g. C<me> for top-level, >+or the relation name for nested embeds like C<biblio>). >+ >+Returns the scalar ref, or undef if the relationship has no DBIC metadata. >+ >+=cut >+ >+sub _build_count_subquery { >+ my ( $rel, $result_set, $parent_alias ) = @_; >+ >+ 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\.//; >+ >+ return \"(SELECT COUNT(*) FROM $rel_table WHERE $rel_table.$fk_foreign = $parent_alias.$fk_self)"; >+} >+ > sub _parse_prefetch { >- my ( $key, $embed, $result_set ) = @_; >+ my ( $key, $embed, $result_set, $attributes, $parent_alias, $count_aliases ) = @_; >+ >+ $parent_alias //= 'me'; > > my $pref_key = $key; > $pref_key =~ s/_count$// if $embed->{$key}->{is_count}; > return unless exists $result_set->prefetch_whitelist->{$pref_key}; > >+ # Handle nested +count embeds with SQL-level subquery >+ if ( $embed->{$key}->{is_count} && $attributes ) { >+ my $subquery = _build_count_subquery( $pref_key, $result_set, $parent_alias ); >+ if ($subquery) { >+ >+ # Use the full dotted path as the alias key so order_by fixup >+ # can match e.g. "biblio.items_count" from _from_api_param >+ my $alias_key = $parent_alias eq 'me' ? $key : "$parent_alias.$key"; >+ push @{ $attributes->{'+select'} }, $subquery; >+ push @{ $attributes->{'+as'} }, $alias_key; >+ $count_aliases->{$alias_key} = $subquery if $count_aliases; >+ return; # no prefetch needed, count is in the subquery >+ } >+ } >+ > my $ko_class = $result_set->prefetch_whitelist->{$pref_key}; > return $pref_key unless defined $embed->{$key}->{children} && defined $ko_class; > > my @prefetches; > foreach my $child ( sort keys( %{ $embed->{$key}->{children} } ) ) { >- my $parsed = _parse_prefetch( $child, $embed->{$key}->{children}, $ko_class->new ); >+ my $parsed = _parse_prefetch( >+ $child, $embed->{$key}->{children}, $ko_class->new, $attributes, $pref_key, >+ $count_aliases >+ ); > push @prefetches, $parsed if defined $parsed; > } > >-- >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
|
194338
|
194339
|
194341
|
194342