Skip to content

fix: aggregate each record once when a filter joins a to-many relationship - #12

Draft
wtsnz wants to merge 1 commit into
mainfrom
fix/filter-fanout
Draft

wtsnz wants to merge 1 commit into
mainfrom
fix/filter-fanout

Conversation

@wtsnz

@wtsnz wtsnz commented Sep 26, 2026

Copy link
Copy Markdown
Owner

The problem

On AshPostgres, an aggregate whose own filter references a to-many relationship counts each record once per matching related row:

# Two comments with 2 likes match `ratings.score > 5`, the first through two ratings.
Post
|> Ash.Query.aggregate(:result, :count, :comments, query: [filter: expr(ratings.score > 5)])
|> Ash.read_one!()
# => result: 3  (expected 2)

# sum of likes => 6 (expected 4); list => [2, 2, 2] (expected [2, 2])

or filters across a relationship and an attribute double-count as well. The wrong numbers come back without any error.

Why

The lateral strategy applies the aggregate's filter with a join into the aggregate's subquery:

SELECT sc0.post_id, count(*) FROM comments AS sc0
INNER JOIN comment_ratings AS sc1 ON sc0.id = sc1.resource_id
WHERE p0.id = sc0.post_id AND sc1.score > $1
GROUP BY sc0.post_id

Each comment then appears once per matching rating before count, sum, avg, list or a custom aggregate runs.

The fix

When the aggregate's filter references a to-many relationship, it is now evaluated inside exists over the aggregated resource, correlated by primary key:

... FROM comments AS sc0
WHERE p0.id = sc0.post_id
  AND exists(SELECT ... FROM comments AS ssc0
             INNER JOIN comment_ratings AS ssc1 ON ssc0.id = ssc1.resource_id
             WHERE ssc0.id = sc0.id AND ssc1.score > $1)
  • Each record once: each record is aggregated once, however many related rows match.
  • Same related row: the whole filter stays in one exists. Conditions on one relationship, such as ratings.score > 8 and ratings.score < 10, still have to hold for the same related row, as Ash's filter semantics require. Rewriting each condition as its own exists would break that.
  • Existing translation reused: it uses the exists-over-a-resource translation AshSQL already has, which sets the tenant and actor from the outer query.

It applies to root aggregates and to aggregates over a direct relationship. These are unchanged:

  • filters that use parent/1, since their references would need to move one level out;
  • aggregates over a multi-hop path;
  • aggregates grouped with others, which don't happen here, since can_group? already keeps to-many filters in their own subquery.

Tests

The regression tests are in AshPostgres: wtsnz/ash_postgres#7. They check count, sum and list through a to-many filter, two conditions that must hold for the same related row, and an or across the relationship and an attribute.

  • On the published AshSQL they fail: count 3 instead of 2, and the or sum 13 instead of 11.
  • They pass with this branch (ASH_SQL_VERSION=local).

Also checked:

  • AshSQL's own tests pass (20).
  • AshPostgres's full suite fails exactly the same 39 tests as main in the local test database, and nothing new.
  • AshSqlite's full suite passes (278).

Found by

The data-layer conformance suite (wtsnz/ash#7), gap filter-fanout. With this branch, all 9 Postgres scenarios behind it pass (count, sum, avg, list, custom, AND, OR, a record count and a composite-key count), and no other result changes on Postgres or SQLite.

…nship

An aggregate whose own filter references a to-many relationship joined
the related rows into the lateral subquery, so each aggregated record was
counted once per matching related row: two comments with three matching
ratings summed to 6 instead of 4, and an OR across the relationship and
an attribute double-counted too.

Evaluate such a filter inside exists over the aggregated resource,
correlated by primary key. Each record is aggregated once, and the whole
filter stays in one exists, so conditions on one relationship still hold
for the same related row. Filters that use parent/1 are left unchanged,
as their references would need to move one level out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant