Skip to content

Commit ee54634

Browse files
committed
fix: ensure limits/offsets are applied in all aggregates
1 parent 4437706 commit ee54634

2 files changed

Lines changed: 82 additions & 15 deletions

File tree

lib/aggregate.ex

Lines changed: 80 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -517,31 +517,40 @@ defmodule AshSql.Aggregate do
517517
tenant,
518518
join_filters
519519
) do
520+
limited? = limited_relationship?(first_relationship)
521+
520522
AshSql.Join.related_subquery(
521523
first_relationship,
522524
tmp_query,
523525
start_bindings_at: start_bindings_at,
524526
refs_at_path: root_data_path,
525527
skip_distinct_for_first_rel?: true,
528+
sort?: limited?,
526529
on_subquery: fn subquery ->
527530
base_binding = subquery.__ash_bindings__.root_binding
528531
current_binding = subquery.__ash_bindings__.current
529532

530533
subquery =
531-
subquery
532-
|> Ecto.Query.exclude(:select)
533-
|> Ecto.Query.select(%{})
534-
535-
subquery =
536-
apply_relationship_subquery(
537-
subquery,
538-
first_relationship,
539-
query,
540-
tenant,
541-
source_binding,
542-
current_binding,
543-
base_binding
544-
)
534+
if limited? do
535+
apply_limited_relationship_subquery(
536+
subquery,
537+
first_relationship,
538+
source_binding,
539+
base_binding
540+
)
541+
else
542+
subquery
543+
|> Ecto.Query.exclude(:select)
544+
|> Ecto.Query.select(%{})
545+
|> apply_relationship_subquery(
546+
first_relationship,
547+
query,
548+
tenant,
549+
source_binding,
550+
current_binding,
551+
base_binding
552+
)
553+
end
545554

546555
subquery =
547556
AshSql.Join.set_join_prefix(
@@ -602,6 +611,63 @@ defmodule AshSql.Aggregate do
602611
)
603612
end
604613

614+
# Relationships that declare a `limit` (or `offset`) need the limit applied
615+
# to the correlated rows *before* the aggregation's `GROUP BY`, otherwise the
616+
# limit caps the number of groups (always 1 in a lateral join) instead of the
617+
# number of rows per group.
618+
defp limited_relationship?(relationship) do
619+
(is_integer(Map.get(relationship, :limit)) or
620+
(Map.get(relationship, :offset) || 0) > 0) and
621+
is_nil(Map.get(relationship, :manual)) and
622+
!Map.get(relationship, :no_attributes?) and
623+
relationship.type != :many_to_many
624+
end
625+
626+
# Builds:
627+
#
628+
# SELECT ... FROM (
629+
# SELECT * FROM destination
630+
# WHERE destination.destination_attribute = parent.source_attribute
631+
# ORDER BY <relationship sort> LIMIT <relationship limit>
632+
# ) AS <base_binding>
633+
# GROUP BY destination_attribute
634+
#
635+
# The correlation, sort and limit all live in the inner subquery so that the
636+
# limit bounds the rows per parent, and the aggregate functions fold the
637+
# already-limited rows.
638+
defp apply_limited_relationship_subquery(subquery, rel, source_binding, base_binding) do
639+
field = rel.destination_attribute
640+
641+
inner =
642+
from(row in subquery,
643+
where:
644+
field(
645+
parent_as(^source_binding),
646+
^rel.source_attribute
647+
) ==
648+
field(
649+
as(^base_binding),
650+
^rel.destination_attribute
651+
)
652+
)
653+
654+
inner =
655+
case Map.get(rel, :limit) do
656+
limit when is_integer(limit) -> Ecto.Query.limit(inner, ^limit)
657+
_ -> inner
658+
end
659+
660+
from(row in subquery(inner), as: ^base_binding)
661+
|> Map.put(:__ash_bindings__, subquery.__ash_bindings__)
662+
|> Ecto.Query.select(%{})
663+
|> then(fn wrapped ->
664+
from(row in wrapped,
665+
group_by: field(row, ^field),
666+
select_merge: %{^field => field(row, ^field)}
667+
)
668+
end)
669+
end
670+
605671
defp apply_relationship_subquery(
606672
subquery,
607673
%{manual: {module, opts}} = rel,

lib/join.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,8 @@ defmodule AshSql.Join do
400400
if opts[:return_subquery?] do
401401
subquery(query)
402402
else
403-
if Enum.empty?(query.joins) && Enum.empty?(query.order_bys) && Enum.empty?(query.wheres) do
403+
if Enum.empty?(query.joins) && Enum.empty?(query.order_bys) && Enum.empty?(query.wheres) &&
404+
Enum.empty?(query.group_bys) && is_nil(query.limit) do
404405
query
405406
else
406407
from(row in subquery(query), as: ^(opts[:start_bindings_at] || 0))

0 commit comments

Comments
 (0)