Add Aggregate Support using ash_sql implementation - #5
Draft
wtsnz wants to merge 12 commits into
Draft
Conversation
7 tasks
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.. |
7 tasks
wtsnz
force-pushed
the
feat/sqlite-aggregate-support-from-ash-sql
branch
2 times, most recently
from
July 22, 2026 01:17
5eae740 to
4e352d2
Compare
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
force-pushed
the
feat/sqlite-aggregate-support-from-ash-sql
branch
from
July 22, 2026 02:58
4e352d2 to
f4f261c
Compare
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.
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
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.
Adds support for aggregates using the new grouped aggregates implementation in ash_sql.