Skip to content

fix: order uniq list aggregates by their cast field - #277

Closed
pcapel wants to merge 1 commit into
ash-project:mainfrom
pcapel:fix/uniq-list-aggregate-sort-cast
Closed

pcapel wants to merge 1 commit into
ash-project:mainfrom
pcapel:fix/uniq-list-aggregate-sort-cast

Conversation

@pcapel

@pcapel pcapel commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

A list aggregate with uniq? and a sort on its own field fails when that field's column is varchar:

ERROR 42P10 (invalid_column_reference) in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list

On this path (a3e39b1 keeps it for sorts on the aggregated field), the aggregated value is cast to the aggregate's type, but the ORDER BY uses the bare column:

array_agg(DISTINCT sc0."title"::text ORDER BY sc0."title" ASC NULLS LAST)

On a text column the cast is a no-op, so the two expressions match and the existing tests pass. On varchar it's a real conversion, so they don't. This change orders by the same field expression the aggregate uses, keeping each sort's direction:

array_agg(DISTINCT sc0."label"::text ORDER BY sc0."label"::text ASC NULLS LAST)

Sorts on other fields still take the dedupe_sorted? path and aren't affected.

The regression test is in ash-project/ash_postgres#906. It fails with 42P10 on main and passes with this change.

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

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session-Id: 32c42430-6f51-4fb3-901e-04112f8b59fd
@pcapel

pcapel commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Failing CI run appears to be something that made its way onto main. I'm going to presume that is something the team is aware of and rebase onto a fix if it becomes available.

@pcapel
pcapel marked this pull request as ready for review October 23, 2026 15:54
zachdaniel added a commit that referenced this pull request Oct 24, 2026
@zachdaniel zachdaniel closed this Oct 24, 2026
@zachdaniel

Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

Decided to do it slightly differently, but based on your solution. Thanks!

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