Skip to content

Add support for Aggregates#213

Open
wtsnz wants to merge 7 commits into
ash-project:mainfrom
wtsnz:feat/sqlite-aggregate-support
Open

Add support for Aggregates#213
wtsnz wants to merge 7 commits into
ash-project:mainfrom
wtsnz:feat/sqlite-aggregate-support

Conversation

@wtsnz

@wtsnz wtsnz commented May 20, 2026

Copy link
Copy Markdown

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

This is my first proper dive into extending the Ash ecosystem, so I’m still getting my bearings with some of the internals and conventions.

I needed aggregates for a local app I’m building on SQLite, and spent the last couple of 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.

I took a look at what AshPostgres supports - the tests helped a lot to get an idea of what to support - and tried to get most of it working in SQLite.

For what it's worth I created this PR with a lot of help from AI; Codex and Claude.

Summary

This covers the aggregate kinds I most wanted to use day-to-day:

  • count
  • sum
  • avg
  • min
  • max
  • exists
  • first
  • list
  • SQLite-compatible custom aggregates

This also adds AshSqlite.CustomAggregate that follows the same general idea as AshPostgres.CustomAggregate - you've just got to make sure that you emit SQLite-compatible SQL.

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 this different approach requires us to add our own SQLite-specific aggregate planner in AshSqlite.Aggregate.

The implementation builds aggregate queries as grouped subqueries or windowed subqueries, then joins those results back to the parent query. Scalar aggregates can often share grouped subqueries. first and list need window/query handling so ordering stays deterministic, and list uses SQLite JSON aggregation before being decoded back into the loaded aggregate value.

A lot of the work here is defensive so that AshSqlite does not generate SQL that returns subtly wrong results. Aggregate filters that join across to-many relationships can accidentally multiply rows and over-count or over-sum. For those cases, I've tried to either use a safe shape or return an explicit unsupported error. I've made sure to add enough tests to be confident in the implementation.

Future potential

This pr is intentionally SQLite-scoped rather than full AshPostgres parity, and already does pretty much everything I need for my project so far.

The main things I’ve left out are cases where SQLite would need a very different query shape, or where the generated SQL could quietly return the wrong answer. That includes things like parent-dependent aggregate filters, manual relationships, some mixed multi-hop/many-to-many paths, and fanout-prone filters over to-many relationships.

Those cases should fail with explicit unsupported errors rather than producing over-counted or over-summed results. The docs include the fuller list of current limitations.

Even more future

After quickly posting a concept of this to the Ash Discord, Zach brought up an interesting idea for a future refactor of aggregates support so that it lives in the ash_sql:

Something that would be cool to do is actually get this and ash_postgres' aggregate logic combined into ash_sql, gated on whether or not the underlying sql implementation supports lateral joins or not.

I like the idea, and am open to taking a look after it gets landed in ash_sqlite.

Validation

  • MIX_ENV=test mix test.reset
  • MIX_ENV=test mix test

@zachdaniel

Copy link
Copy Markdown
Contributor

@wtsnz the audit check can be ignored, but mind looking into the dialyzer issues?

@wtsnz

wtsnz commented May 20, 2026

Copy link
Copy Markdown
Author

Alright @zachdaniel I think I've got it - could you run the CI again please?

@zachdaniel

Copy link
Copy Markdown
Contributor

@wtsnz looks like just some reuse errors and compile warnings now.

@wtsnz wtsnz force-pushed the feat/sqlite-aggregate-support branch from a7e1639 to 28b3b48 Compare June 5, 2026 19:14
@wtsnz

wtsnz commented Jun 5, 2026

Copy link
Copy Markdown
Author

Alright @zachdaniel - third time's the charm - could you run it again please?

Also before we merge this, do you have any thoughts on this -

There's two possible approaches to add aggregates into AshSqlite -

  1. Merge this PR in
    a. Add support for Aggregates #213
  2. Use the newer approach that adds aggregate strategies to ash_sql and updates ash_sqlite to implement them)
    a. Add multiple aggregate strategies to ash_sql (Add aggregate strategies support (lateral & grouped) wtsnz/ash_sql#1)
    b. Add support for aggregates in ash_sqlite via the (a) Add Aggregate Support using ash_sql implementation wtsnz/ash_sqlite#5
    c. Also a tiny pr to ash_postgres that explicitly states the aggregate_strategy (wtsnz/ash_postgres@c02e86f)

Let me know which way you'd like to take it. For what it's worth I've been using the ash_sql aggregate version (2) in my local app to great success!

@zachdaniel

Copy link
Copy Markdown
Contributor

Ah, right, I like the ash_sql based approach 😄 Lets go with that one instead.

@zachdaniel

Copy link
Copy Markdown
Contributor

Those PRs are drafts, would you say they are ready for review?

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.

2 participants