Skip to content

dev#256

Merged
matt-beanland merged 8 commits into
mainfrom
dev
May 10, 2026
Merged

dev#256
matt-beanland merged 8 commits into
mainfrom
dev

Conversation

@matt-beanland
Copy link
Copy Markdown
Collaborator

No description provided.

Aggregate filters declared via `filter expr(...)` in the Ash DSL were
never applied — they were silently ignored by every code path in
run_aggregate_for_ids.

## Root cause

All three aggregate execution paths (Cypher push-down for plain fields,
Elixir-side for embedded JSON types, Elixir-side for expression
calculations) received `aggregate.filter` but never consulted it.

## Fix

Routing in run_aggregate_for_ids/5 is now:

1. Expression-calc field (Ash.Query.Calculation) → run_expr_agg, which
   now applies the filter via apply_record_filter/3 before evaluating
   the expression.

2. Any filter present on a plain or embedded aggregate →
   run_filtered_aggregate/7 (new): loads full destination records via
   related_nodes, applies Ash.Filter.Runtime.filter_matches/3 per-source
   group, extracts field values, then reduces with apply_elixir_aggregate.

3. Embedded field (no filter) → run_embedded_agg (unchanged).

4. Plain field (no filter) → Cypher push-down (unchanged).

This keeps the fast Cypher path for the common unfiltered case while
fully honouring the data-layer contract for filtered aggregates.

## New helpers

- run_filtered_aggregate/7 — Elixir-side aggregate with runtime filter
- extract_aggregate_field_values/2 — extracts field from records, uniq?
- apply_record_filter/3 — applies Ash runtime filter to {id, record} pairs

## Tests

Added describe "filtered aggregates (#252)" covering: first, count,
exists (true and false), list, zero-record base case, and multi-post
correctness. Four filtered resource aggregates added to Post test fixture.
…ntly-dropped

fix #252 aggregate filters silently dropped
…-fail-with-invalid-cypher-identifier-error

backticks and fix for 252
@matt-beanland matt-beanland merged commit 0797e9d into main May 10, 2026
3 checks passed
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