From ed78f9221e711d8b7465089b8419e974d8e5ab2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= 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 Signed-off-by: David Nind --- 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?(?\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 (from a C 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 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. + +Handles both C (plural, via C<_resultset>) and C (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.39.5