Refactor aggregate strategies and add a Grouped strategy - #264
Merged
zachdaniel merged 14 commits intoSep 25, 2026
Merged
zachdaniel merged 14 commits into
zachdaniel merged 14 commits into
Conversation
Move the loaded-aggregate implementation into `AshSql.Aggregate.Lateral` and leave `AshSql.Aggregate` as the compatibility facade. Implementation notes: - preserve the existing public function signatures; - delegate directly to the extracted lateral module; - retain the upstream aggregate read-action correction; - avoid the temporary context wrapper from the earlier history. This is a structural extraction with no intended behavior change.
Move root aggregate-query execution into `AshSql.Aggregate.Lateral.Query` and keep `AshSql.AggregateQuery` as the public entry point. Implementation notes: - preserve the existing aggregate-query API; - normalize bindings once in the facade; - pass the prepared query to the lateral implementation; - remove the duplicate binding initialization from the extracted module. This is a structural extraction with no intended query behavior change.
Introduce the adapter callbacks that select an aggregate planner and provide dialect-specific grouped list expressions. Implementation notes: - `aggregate_strategy/1` defaults to `:lateral`; - adapters can opt into the new `:grouped` strategy; - `grouped_list_aggregate/2` keeps list SQL and result representation in the adapter; - both callbacks remain overridable through `AshSql.Implementation`. Existing adapters continue to use lateral aggregates without any required changes.
Add the grouped and windowed planner used by adapters that cannot load related aggregates through lateral joins. Implementation notes: - apply relationship limits and offsets per parent before aggregation; - resolve attributes, calculations, and aggregate fields through AshSQL expressions; - reuse the existing relationship and sort planners; - preserve aggregate type constraints; - delegate grouped list expressions to the selected adapter. The companion AshSQLite suite covers the supported aggregate kinds, relationship shapes, rich fields, defaults, and bounds.
Route root aggregate queries through the selected strategy and add the grouped implementation for SQLite-style adapters. Implementation notes: - support count, sum, average, minimum, maximum, first, and exists; - apply filters, limits, offsets, and required ordering before aggregation; - preserve `first` nil handling, defaults, and Ash sort semantics; - resolve calculation and aggregate fields through shared expression handling; - derive fieldless distinct counts from single primary-key metadata and reject unsupported key shapes clearly. The companion AshSQLite regressions cover each of these query semantics end to end.
Explain how SQL adapters select between lateral and grouped aggregate planning. Documentation includes: - the default `:lateral` behavior; - the adapter opt-in for `:grouped`; - the adapter-owned grouped list callback; - the new strategy support in the changelog. The guide describes the final contract after both loaded and root aggregate paths are in place.
Normalize resource aggregates and SQL aliases before strategy dispatch. Scope alias reuse and expression lookup to the attachment path, and correlate grouped joins with the supplied source binding. Route nested lateral calls through the shared facade and cover alias reuse, path isolation, and actor/tenant propagation.
Grouped aggregate inputs skipped parts of relationship query preparation. Intermediate and join resources could lose actor, tenant and shared context, ignore configured read actions, or omit required action arguments. Missing attribute-tenant predicates could also count rows belonging to other tenants. Extract the existing relationship preparation boundary for both strategies. Preserve prepared endpoint metadata, apply tenant scope and bypass precedence throughout the path, and group only inputs with matching preparation context. Keep propagated overrides local so sibling aggregates retain their own scope. First/list aggregate filters previously ran before relationship limits and could select rows outside the bounded relationship. Apply read-action scope before bounds and aggregate filters afterward, matching scalar aggregates. Loaded scalar values and defaults also bypassed their declared constrained type. Apply the existing type conversion so custom loaders receive both. Validation: 24 focused SQLite regressions pass, including reproduced failures and controls. Full suites pass: AshSQL 11 tests, SQLite 274 tests, PostgreSQL 2 doctests, 2 properties and 868 tests (21 excluded). Extra-high independent review found no remaining blockers. Existing Sobelow and SQLite test-helper Dialyzer findings remain; corrected local REUSE checks and other gates pass.
7 tasks
Fieldless grouped counts that need distinct records counted only the first primary-key column, or the relationship join attribute when the destination had no primary key. Composite-key and keyless destinations therefore collapsed to one row per parent. Return the same unsupported-key-shape error as grouped query aggregates. Exists aggregates no longer ask for distinct records, since row multiplication cannot change whether any row matched.
Aggregate names are scoped to an attachment path, so one name can be bound at the root and at a related path in the same query. Already-added lateral aggregates were reselected from every aggregate binding with a matching name, letting a later binding overwrite the requested value. Only reselect from bindings at the requested path, the same criterion already_added?/3 uses to find them.
handle_attribute_multitenancy/3 now reads context multitenancy before the read action, which also changes lateral and exists callers that pass no read action. Pin that precedence against Ash.Actions.Read, which resolves read multitenancy the same way, including enforcing contexts that override a bypassing action.
Grouped.add_aggregates/4, add_sort_aggregates/3 and the sort-derived aggregate helpers behind them have no callers; adapters reach grouped planning through the AshSql.Aggregate facade. The unused chain also built aggregate queries without actor or tenant, so leaving it risked reuse.
grouped_list_aggregate/2 must apply the planner's named window itself, and its result must be a JSON list because grouped list defaults are JSON-encoded. Grouped query aggregates also use SQLite's LIMIT -1 for offset-only input. Document these assumptions on the callbacks and in the README rather than implying a dialect-neutral strategy.
git_ops generates CHANGELOG.md from conventional commits at release, so the manual Unreleased section would duplicate or conflict with the generated entries.
Contributor
|
🚀 Thank you for your contribution! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support for multiple aggregate strategies in AshSQL.
I needed aggregates for a local app I’m building on SQLite, and spent a few days exploring the existing SQL adapters and getting this branch into shape with Codex helping lots along the way. My goal was to get most of the common usage of aggregates working with sqlite so that I could continue building my app.
Approach
I originally hoped this could mostly reuse the shared ash_sql aggregate machinery, but the existing aggregate planner is built around SQL shapes SQLite does not support cleanly. Lateral joins and PostgreSQL-style list aggregation are the big ones. For SQLite, the safer route (according to the LLMs!) is grouped/windowed subqueries, JSON aggregation for lists, and explicit errors when a filter shape could multiply the rows being aggregated.
So I've moved that implementation into
AshSql.Aggregate.Lateraland added anAshSql.Aggregate.Groupedstrategy, which builds grouped and windowed subqueries and joins the results back to the parent query.Adapters (ash_sqlite and ash_postgres) choose their strategy through the
aggregate_strategy/1API. Lateral remains the default. I've got another PR in AshSQLite to select the grouped strategy, and provides the SQLite-specific JSON list aggregation expression.I've tried to keep as much shared as possible - aggregate normalization, SQL alias handling, and relationship query preparation. Each strategy handles its own SQL construction. Sharing preparation also keeps actor, tenant, context, and configured read-action arguments available when building aggregate inputs. There were a few gotcha's around here that are now covered by tests.
Contributor checklist
Leave anything that you believe does not apply unchecked.