feat(query): order_by nulls= first/last placement - #369
Merged
Conversation
…#365) ## Summary - Add optional `nulls` on QueryIR `order_by` terms (`"first"` / `"last"`, case-insensitive); omit the key when unset so existing fixtures stay byte-identical at `ir_version` 7 - SELECT walker uses sea-query `order_by_expr_with_nulls` to emit native `NULLS FIRST` / `NULLS LAST` on Postgres and SQLite; absent `nulls` keeps plain `ASC`/`DESC` - Junk tokens (e.g. `"sideways"`) fail loudly as `PyValueError` at plan/render time Closes #361 ## Test plan - [x] `cargo test -p ferro-schema-ir` (fixture round-trips + new wire pin) - [x] `cargo test --no-default-features --features testing` (includes nulls SQL render pins on both dialects) - [ ] CI green on this PR ## Exit steps - [x] Issue status updates encoded in PR body (`Closes #361`) Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Add keyword-only `nulls="first"|"last"` to `Query.order_by` and `ProjectedQuery.order_by` (lambda, string, traversal, aggregate, output alias), with case-insensitive build-time validation matching `direction`. - Thread `nulls` through frozen `OrderByEntry`; `to_ir_dict` emits `"nulls"` only when set (omit when unset). `ir_version` stays 7. - Stops at `compile_query` — no Rust SELECT renderer, no golden IR vectors, no live-DB e2e (those are #361/#363). Closes #362 ## Test plan - [x] `uv run pytest tests/test_query_column_validation.py tests/test_order_by_nulls_wire.py -q` - [x] `nulls=` accepted on Query/ProjectedQuery; keyword-only third positional is `TypeError` - [x] Junk `nulls` is `ValueError` naming `first`/`last`; case-insensitive `FIRST`/`LAST` ok - [x] Omitted `nulls=` keeps legacy clause equality / no `nulls` key on wire - [x] `compile_query` emits `"nulls"` only on set terms; `ir_version == 7` ## Exit steps - [x] Issue #362 auto-closed via `Closes #362` in this PR body - [x] Assignee claimed on issue Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Hand-authored `query_card_nulls_v7` golden vector (first `order_by` term with `nulls`, next without; `ir_version` 7) asserted from the Python emitter and Rust decoder; existing vectors unchanged. - Backend-matrix e2e (`tests/test_order_by_nulls.py`) pins identical row order on SQLite and Postgres for `DESC`+`nulls="last"`, `nulls="first"`, chained terms, left-joined related `NOT NULL` columns, and projected `SUM` aggregates with NULL empty groups. - Contract validator accepts optional `nulls` and restricts present values to `"first"`/`"last"`. Closes #363 ## Test plan - [x] `uv run pytest tests/test_order_by_nulls.py tests/test_query_wire_vectors.py tests/test_ir_vectors_contract.py -q` - [x] `cargo test -p ferro-schema-ir` - [ ] CI backend matrix (sqlite + postgres) ## Exit steps - [x] Issue status: `Closes #363` in PR body; assignee claimed Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Document `order_by(..., nulls="first" | "last")` in the Queries guide with a pinned-first `Card` example (assignment + Annotated model tabs) and the rendered `NULLS LAST` SQL. - State once that omitting `nulls=` on a nullable sort key is dialect-defined (Postgres `DESC` → NULLs first; SQLite `DESC` → NULLs last); widen the former `left_join`-only note to cover nullable root columns and point at `nulls=`. - Mention the kwarg in the Queries API page; extend runnable `docs/examples/predicates*.py` companions. Closes #364 ## Test plan - [x] `uv run pytest tests/test_docs_examples.py -q` (172 passed) - [x] `uv run python docs/examples/predicates.py` - [x] `uv run python docs/examples/predicates_annotated.py` - [ ] Confirm issue #364 closes on merge into the parent stack Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Gate pass (coordinator). Full integration vs |
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
1 participant
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.
Summary
Pin
NULLsort-key placement on everyorder_by()withnulls="first" | "last"(keyword-only). NativeNULLS FIRST/NULLS LASTon Postgres and SQLite. Omit the kwarg → backend default, no warning.nullson an order_by term; key omitted when unsetQueryandProjectedQuery(lambda, string, traversal, aggregate, output alias)Stacked: #365 (IR/SQL) → #366 (API) → #367 (e2e) → #368 (docs)
Closes #358
Closes #361
Closes #362
Closes #363
Closes #364
Test plan
mainfeatPRD:order_byNULLS FIRST / NULLS LAST placement #358,featorder_by nulls 1/4: native NULLS clause from QueryIR #361–featorder_by nulls 4/4: docs #364 close on merge tomainMade with Cursor