feat(query): position paging after()/before() from order_by - #404
Merged
Conversation
Pin Paging, Position, Order key, and Null placement, plus ADR-0017/0018, ahead of the #372 implementation slices. Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Omitted `nulls=` on `order_by` now compiles to **`last`** on every backend (breaking change: Postgres DESC and SQLite ASC no longer use dialect defaults). - New explicit escape: `nulls="native"` restores plain ASC/DESC per dialect. - QueryIR **11 → 12**: every `order_by` term carries required `nulls` (`first` | `last` | `native`); missing key fails at decode. ## Breaking change | Before (omitted `nulls=`) | After | |---|---| | Postgres DESC → NULLs first | NULLs last | | SQLite ASC → NULLs first | NULLs last | Use `nulls="native"` when you want the old dialect-default behavior. ## Test plan - [x] `pytest tests/test_order_by_nulls.py` — omitted `nulls=` cross-asserted as last on SQLite - [x] `pytest tests/test_order_by_nulls_wire.py` — v12 wire, every term has `nulls`, `native` token - [x] `pytest tests/test_query_wire_vectors.py` — golden vectors at ir_version 12 - [x] `pytest tests/test_ir_vectors_contract.py` — contract requires `nulls` on every order_by term - [x] `cargo test -p ferro-schema-ir query_order_by query_card_nulls` — required nulls decode/roundtrip - [x] `pytest tests/test_query_column_validation.py` — builder accepts `native`, omitted → `last` Closes #392 Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - `after(position)` pages a query whose order keys are **root columns**, include the **primary key**, and are all **non-nullable**. The bound is exclusive. `position_of(row)` reads that tuple; `after(row)` is sugar for `after(position_of(row))`. - QueryIR **v13**: fetch payloads carry one optional `after` bound as typed `kind`/`value` nodes (omitted when unset). `count()` drops paging the same way it drops limit/offset; `update`/`delete` reject `after`. `after` + `offset` is a build-time error (two starts). - Rust expands one exclusive stepwise compare — `(a > :a) OR (a = :a AND b > :b)`, DESC flipping the inequality — so #394 can extend that function. Datetime query binds use pydantic JSON (`…Z` for UTC) so SQLite TEXT equality matches INSERT. ## Test plan - [x] Build-time: no PK in order keys; PK-less model; nullable order key; wrong arity; `None` in a slot; `after`+`offset`; `update`/`delete` reject `after`; `count()` omits `after` - [x] e2e sqlite: `order_by(updated_at).order_by(id).after((ts, id)).limit(n)` returns the next n exclusive, in declared order; empty page past the end; `after(row)` equals `after(position_of(row))` - [x] Golden vector `query_user_after_v13` plus all query vectors bumped to `ir_version` 13 - [x] Rust: exclusive stepwise compare ASC and DESC on two NOT NULL keys; QueryIR accepts v13 - [ ] Postgres e2e locally (pytest-postgresql could not start; no `FERRO_POSTGRES_URL`) Closes #393 Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - `after()` now accepts `None` in every non-PK position slot (PK `None` is still a build-time error) and no longer consults `ColumnSpec.nullable`. - `exclusive_stepwise_compare` is still the one expander: each key's direction, `nulls=` placement, and whether the bound is NULL fold into one `IS NULL` / `IS NOT NULL` / inequality tree so a cursor can cross the NULL bucket in one query. - `"native"` resolves at render from `Dialect` (Postgres: NULL is larger; SQLite: NULL is smaller). QueryIR stays at v13; a new golden pins `kind: "null"` in `after`. Closes #394 ## Test plan - [x] `uv run pytest tests/test_after_position.py` — build-time contract + SQLite backend-matrix e2e (Pinch `nulls="last"` DESC, `after((None, id))`, non-NULL cursor into the NULL bucket) - [x] `cargo test --no-default-features --features testing query::` — expander combinatorics (ASC/DESC × first/last × bound-NULL/non-NULL, native on both dialects, two keys) - [x] Wire/IR: `tests/test_query_wire_vectors.py`, `tests/test_ir_vectors_contract.py`, `cargo test -p ferro-schema-ir query_after` - [x] `uv run ty check src/ferro/query` - [ ] Postgres backend-matrix — `pytest-postgresql` could not start locally (`pg_ctl initdb` failed). CI will run it. Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Description `before(position)` is now the other paging start. `before(pos).limit(n)` is the adjacent previous page in declared order; unbounded `before(pos).all()` is every earlier row (a prefix). The bound is exclusive. On unbounded `before()`, `first()` (limit 1 → adjacent previous row) and `all()[0]` (prefix head) disagree. That is accepted and documented. ## Changes - Invert each order key (`asc`↔`desc`, `first`↔`last`, `native` stays `native`), reuse `#394`'s `exclusive_stepwise_compare`, fetch, then reverse hydrated rows. - QueryIR v13 → v14 (`before` omitted when unset). `after` + `before` and `before` + `offset` are build-time errors; mutations reject `before` the same way they reject `after`. - Same order-key rules as `after()`: root columns, PK included, `None` legal in every non-PK slot. Traversed keys stay #396. ## Test plan - [x] `tests/test_before_position.py`: adjacent page, unbounded prefix, `first()` vs `all()[0]`, after+before / offset+before errors, NULL-bucket Pinch, `before(row)` sugar - [x] Rust `query::` pins invert + same expander (no second tree); `native` unchanged - [x] Wire/goldens at v14 (`query_user_before_v14.json`); `rejects_v13`; future-reject is 15 - [x] sqlite e2e (backend matrix) - [ ] Postgres e2e — skipped locally (`pytest-postgresql` could not start `initdb`); CI will run it - [x] `uv run ty check src/ferro/query` ## Bridge and Schema Impact - [ ] No Rust/Python bridge changes - [ ] Python model/schema changed - [x] Rust core or SQL generation changed - [ ] `src/ferro/_core.pyi` updated (if needed) - [x] Integration test added first for new behavior ## Migration / Breaking Changes - [x] No breaking changes - [ ] Breaking changes included (details below) QueryIR 13 → 14 is an in-wheel wire bump (Python and Rust ship together). ## Documentation and Changelog - [ ] No docs update needed - [x] Docs updated (README/docs/inline docs) - [ ] Changelog entry needed I-10: changelog is release-managed; do not edit `CHANGELOG.md`. ## Related Issues Closes #395 ## Exit Steps - [x] Related completed issues are linked above with Closes/Fixes/Resolves - [x] Issue statuses are updated/auto-close on merge --------- Co-authored-by: Cursor <cursoragent@cursor.com>
## Description `after` / `before` now page when an order key is a related column (`order_by(lambda t: t.account.label)`) or the query is a projected record. A decoded tuple is enough — `include()` is not required to page. `position_of` on a model instance requires those relations populated (same contract as reading `row.account.label`). `position_of` on a projected `Row` requires every order key in the projection; otherwise pass a tuple. ## Changes - Lift the Python path refusal in `_assert_position_order_keys`. Root PK must still appear among the order keys (a related `id` does not count). `None` stays legal in every non-PK slot, including a `left_join`'d missing relation. - `position_of(instance)` walks populated `__dict__` relation hops and errors by path when a hop is unpopulated. - `ProjectedQuery.position_of(Row)` matches order keys by source or output name; missing key tells the caller to pass a tuple. - Aggregate order keys error even when the PK is a group key. Grouped aggregates without the PK still fail the existing PK-in-order-keys rule. - Rust pin: `before` on a path-carrying order key still qualifies through `qualify_column_with_joins` (no second expander). ## Bridge and Schema Impact - [ ] No Rust/Python bridge changes - [x] Python model/schema changed - [x] Rust core or SQL generation changed - [ ] `src/ferro/_core.pyi` updated (if needed) - [x] Integration test added first for new behavior Python builder/validation only plus a Rust unit pin. QueryIR is unchanged (`ir_version` stays 14). Path-carrying `order_by` already qualified in the walker. ## Migration / Breaking Changes - [x] No breaking changes - [ ] Breaking changes included (details below) Queries that previously raised "traversed order key is not supported yet" now page. ## Documentation and Changelog - [ ] No docs update needed - [x] Docs updated (README/docs/inline docs) - [ ] Changelog entry needed User-visible `feat` — release tooling records the changelog (I-10). Do not hand-edit `CHANGELOG.md`. ## Related Issues - Closes #396 ## Test plan - [x] sqlite e2e: traversed `after`/`before` from a tuple (no `include()`), `left_join` + `None` related slot, projected-record paging, `position_of` populated vs unpopulated - [x] build-time: unpopulated path, missing projection key, grouped aggregate (PK rule), aggregate order key, related `id` is not the model PK, expression order keys still fail at `order_by()` - [x] `cargo test --no-default-features --features testing query::` (includes `before_condition_qualifies_path_carrying_order_key`) - [x] `uv run ty check src/ferro/query` - [ ] Postgres backend matrix — `pytest-postgresql` cannot start locally (`pg_ctl initdb` failed). CI will run it. ## Exit Steps - [x] Related completed issues are linked above with Closes/Fixes/Resolves - [x] Issue statuses are updated/auto-close on merge Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
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
Keyset
after()/before()derived from a query'sorder_by, NULL-placement aware (ADR-0018). A position is the ordered tuple of order-key values;position_of(row)reads it;after(row)/before(row)are sugar. Cursor encoding stays the caller's.nulls=is"last"on every dialect (ADR-0017 — breaking vs native PG DESC / SQLite ASC)."native"is the explicit escape.after(position)pages forward (exclusive).Noneis legal in every non-PK slot so a pinned-first list can cross into unpinned rows in one query.before(position).limit(n)is the adjacent previous page in declared order; unboundedbefore()is the earlier-row prefix (first()andall()[0]disagree — documented).include()not required).position_ofstill requires populated relations / selected keys.QueryIR is v14. One Rust expander (
exclusive_stepwise_compare) owns the compare tree;beforeinverts keys then reuses it.Test plan
feat/position-paging: feat(query): omitted order_by nulls= means last #397–feat(query): after/before on traversed keys and projections #403 (rust, python PR, sqlite+postgres matrix)mainorder_by(pinned_at, "desc").order_by(id).after((None, id))andbefore(pos).limit(n)Closes #372
Made with Cursor