Skip to content

Add Aggregate Support using ash_sql implementation - #5

Draft
wtsnz wants to merge 12 commits into
mainfrom
feat/sqlite-aggregate-support-from-ash-sql
Draft

Add Aggregate Support using ash_sql implementation#5
wtsnz wants to merge 12 commits into
mainfrom
feat/sqlite-aggregate-support-from-ash-sql

Conversation

@wtsnz

@wtsnz wtsnz commented May 21, 2026

Copy link
Copy Markdown
Owner

Adds support for aggregates using the new grouped aggregates implementation in ash_sql.

@wtsnz

wtsnz commented May 22, 2026

Copy link
Copy Markdown
Owner Author

hmmm ash_postgres doesn't have any docs on aggregates, maybe I should remove from here - they're documented in ash itself - though maybe it would be worth having a small note on what is not supported..

@wtsnz
wtsnz force-pushed the feat/sqlite-aggregate-support-from-ash-sql branch 2 times, most recently from 5eae740 to 4e352d2 Compare July 22, 2026 01:17
wtsnz added 4 commits July 21, 2026 19:48
Problem:
The grouped `first` implementation had no regression coverage for nil exclusion, explicit defaults, or the difference between Ash and Ecto null-ordering atoms.

Change:
Exercise `include_nil?` in both states, an empty filtered result with a fallback, ascending and descending order, and all four null-ordering variants.

Provenance:
This coverage protects the `first` correctness defect introduced by the new grouped query aggregate implementation.
Problem:
The grouped fieldless distinct count assumed every resource had an `:id` attribute. There was no coverage for valid named keys or for unsupported zero-key and composite-key resources.

Change:
Add a read-only test resource whose primary key maps to the posts table under a different Ash name. Verify that it counts correctly and that zero-key and composite-key resources return clear errors. Document the single-key requirement.

Provenance:
This coverage protects the hard-coded `:id` defect introduced by the new grouped query aggregate implementation.
Problem:
The earlier SQLite aggregate implementation discarded constraints when it converted aggregate item types. A normal value assertion would not reliably prove that the generated Ecto type retained those constraints.

Change:
Build the grouped aggregate data-layer query and inspect it for the constrained string Ecto type. Assert that `trim?` and `allow_empty?` survive query construction.

Provenance:
This covers a defect carried from the earlier SQLite implementation rather than a regression introduced by the extraction.
Problem:
An aggregate over an ordered query with only an offset could aggregate the full input instead of the offset result set. SQLite also requires a limit expression when SQL contains an offset.

Change:
Create three ordered records, offset the first record, and assert that the aggregate count is two. Document that root query limits and offsets are applied before aggregation.

Provenance:
This covers behavior inherited from the existing AshSQL query-wrapping condition and copied into the grouped strategy.
@wtsnz
wtsnz force-pushed the feat/sqlite-aggregate-support-from-ash-sql branch from 4e352d2 to f4f261c Compare July 22, 2026 02:58
Problem:
The grouped implementation had no SQLite regressions for relationship bounds, rich aggregate fields, or limited root first and exists queries. Its shared AshSQL module also owned SQLite's json_group_array expression directly.

Change:
Add focused resources and tests for limit, offset, calculation fields, aggregate-on-aggregate fields, and limited query input. Implement the adapter callback that supplies SQLite's JSON-backed windowed list aggregate.

Why:
The tests demonstrate each reported issue before the AshSQL fix and protect the richer Ash value shapes Zach highlighted in ash_sqlite PR ash-project#214. Keeping SQLite SQL in AshSqlite preserves the shared strategy boundary.

Provenance:
The covered defects and adapter-boundary issue were introduced by the grouped aggregate extraction.

Validation:
All four focused regressions pass, followed by the complete AshSQLite suite with 210 passing tests against the local AshSQL branch.
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