Skip to content

fix: sort a ranked union in ETS, not only a ranked union_all - #2828

Merged
zachdaniel merged 2 commits into
ash-project:mainfrom
matt-beanland:fix/ets-ranked-union-sort
Aug 3, 2026
Merged

fix: sort a ranked union in ETS, not only a ranked union_all#2828
zachdaniel merged 2 commits into
ash-project:mainfrom
matt-beanland:fix/ets-ranked-union-sort

Conversation

@matt-beanland

Copy link
Copy Markdown
Contributor

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

Summary

Fixes #2827 .

Sorting by a combination calculation did nothing when the parts were joined with union. The
same query with union_all sorted correctly. One record, reached by three parts ranking it
1/2/3, sorted descending:

union_all -> [3, 2, 1]     correct
union     -> [1, 2, 3]     unsorted

That is the ranked shape the combinations guide documents. Repro in the issue.

Cause

run_query/3 in lib/ash/data_layer/ets/ets.ex tells Ash.Actions.Sort.runtime_sort/3 whether
the fold can return the same primary key twice:

maybe_not_distinct? = Enum.any?(combination_of, &(elem(&1, 0) == :union_all))

runtime_sort/3 uses maybe_not_distinct? when choosing how to load the sort key — batched, or one record at a time. The batched branch cannot resolve two records sharing a primary key into two records, so the key
is mis-assigned and the sort silently does nothing.

The condition is too narrow. union_all is not the only way to get a repeated primary key: for a
union the equality basis is the combination fieldset, so parts carrying different combination
calculations keep both copies.

Now widening to any part carrying combination calculations:

maybe_not_distinct? =
  Enum.any?(combination_of, fn {type, combination} ->
    type == :union_all or not Enum.empty?(combination.calculations)
  end)

Ash.Query.Combination's calculations defaults to %{}, so the emptiness test has to be
Enum.empty?/1. Comparing against [] is true for %{}, which would set the flag for every
combination query and put them all on the unbatched path.

Commits

  1. test: pin the ranked three-part shape from the guide, asserted for union_all - passes as-is.
  2. fix: widen the condition, and assert the same result for union.

`combinations` documents ranking parts with a calculation and sorting the
combined result by it. Nothing pinned that the sort actually happens.

Add the three-part shape the docs describe — one record reached by three
parts that each rank it differently — parameterised on the combination type
so the same helper can be pointed at other types.

Passes as-is for `union_all`; it characterises today's behaviour.
`maybe_not_distinct?` told `Ash.Actions.Sort.runtime_sort/3` whether a
combination fold could return the same primary key twice, and it was set for
`union_all` alone. A `union` does it too: the equality basis is the
combination fieldset, so parts carrying different combination calculations
keep both copies — deliberately, and that is exactly the ranked shape the
combinations guide documents.

Without the flag, `runtime_sort/3` loads sort keys in batches, which cannot
resolve two records sharing a primary key into two records. The key was
mis-assigned and the sort silently did nothing: three parts ranking one
record 1, 2, 3 and sorted descending came back `[1, 2, 3]` under `union` and
`[3, 2, 1]` under `union_all`.

Widen the condition to any part carrying combination calculations.

Note the calculations are a map, not a list, so the emptiness test has to be
`Enum.empty?/1` — comparing against `[]` is true for `%{}` and would force
every combination query onto the unbatched path.
@zachdaniel
zachdaniel merged commit 696e62c into ash-project:main Aug 3, 2026
51 checks passed
@zachdaniel

Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

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.

ETS sorts a ranked union_all but not a ranked union — maybe_not_distinct? is set for union_all only

2 participants