Skip to content

Handle DuckDB's ORDER BY ALL properly - #483

Open
eddietejeda wants to merge 2 commits into
tobilg:mainfrom
hotdata-dev:feat/duckdb-order-by-all
Open

eddietejeda wants to merge 2 commits into
tobilg:mainfrom
hotdata-dev:feat/duckdb-order-by-all

Conversation

@eddietejeda

Copy link
Copy Markdown

ORDER BY ALL is already listed as a known failure category in tests/analyze_failures.rs, so not completely unexpected.

DuckDB parses ALL as a column called all, which has two consequences:

all is in POSTGRES_RESERVED, and DUCKDB_RESERVED is a copy of that set, so the output is:

SELECT a, b FROM t ORDER BY ALL  =>  SELECT a, b FROM t ORDER BY "ALL"

In DuckDB, "ALL" refers to a column named "ALL," so the query no longer runs.

Second, every other target uses the same ORDER BY "ALL", and none accept it.

What this does

  • Removes all from DUCKDB_RESERVED, so DuckDB output keeps the keyword bare.

  • When the source is DuckDB and the target isn't, a lone unquoted ALL in ORDER BY becomes ORDER BY 1, 2, ... over the select list. Every dialect accepts that. Direction and null order are preserved, and it works inside subqueries. DuckDB puts NULLs last even in DESC, so the expansion spells out NULLS LAST, whereas the target would default differently.

  • If the select list is *, we don't know how many columns there are, so it's reported as an unsupported translation instead of emitting SQL that can't run.

    DuckDB -> DuckDB       SELECT a, b FROM t ORDER BY ALL
    DuckDB -> PostgreSQL   SELECT a, b FROM t ORDER BY 1, 2
    DuckDB -> PostgreSQL   ORDER BY ALL DESC  =>  ORDER BY 1 DESC NULLS LAST, 2 DESC NULLS LAST
    DuckDB -> PostgreSQL   SELECT * FROM t ORDER BY ALL  =>  unsupported
    

A quoted "all" in the source is a real column and is left alone.

Tests

tests/duckdb_order_by_all.rs: the DuckDB round trip, expansion for three targets, direction and null order, subqueries, the * error, and the quoted-column case.

This branch has not been deployed

No deployments
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