From 704a6420f8367dbac565bc9e20e9ee68b3947002 Mon Sep 17 00:00:00 2001 From: Taylor Date: Sun, 30 Aug 2026 11:15:43 -0400 Subject: [PATCH 1/4] feat(query): after(position) for non-null root order keys Page a query whose order keys are root columns, include the primary key, and are all non-nullable. Exclusive stepwise compare lives in one Rust function so #394 can extend it; datetime query binds use pydantic JSON so SQLite TEXT equality matches INSERT. Co-authored-by: Cursor --- crates/ferro-schema-ir/src/lib.rs | 99 ++++-- docs/examples/predicates.py | 20 ++ docs/pages/guide/queries.md | 10 +- .../patterns/position-paging-after.md | 23 ++ src/ferro/query/builder.py | 114 +++++++ src/ferro/query/nodes.py | 14 +- src/ferro/query/wire.py | 87 +++-- src/operations.rs | 119 ++++--- src/query.rs | 308 ++++++++++++++---- tests/fixtures/ir_vectors/README.md | 7 +- ...v12.json => query_account_exists_v13.json} | 4 +- ...n => query_account_scoped_exists_v13.json} | 4 +- ...lls_v12.json => query_card_nulls_v13.json} | 4 +- ...son => query_owner_nested_exists_v13.json} | 4 +- ...2.json => query_owner_not_exists_v13.json} | 4 +- ...n => query_transaction_aggregate_v13.json} | 4 +- ...ery_transaction_global_aggregate_v13.json} | 4 +- ...son => query_transaction_include_v13.json} | 4 +- ...n => query_transaction_left_join_v13.json} | 4 +- ...json => query_transaction_record_v13.json} | 4 +- ...n => query_transaction_traversal_v13.json} | 4 +- ...ery_transaction_traversed_record_v13.json} | 4 +- ...on => query_user_add_columns_set_v13.json} | 4 +- ...on => query_user_add_literal_set_v13.json} | 4 +- .../ir_vectors/query_user_after_v13.json | 56 ++++ ..._v12.json => query_user_compound_v13.json} | 4 +- ...2.json => query_user_literal_set_v13.json} | 4 +- ...12.json => query_user_m2m_exists_v13.json} | 4 +- ...v12.json => query_user_merge_set_v13.json} | 4 +- ...v12.json => query_user_mixed_set_v13.json} | 4 +- ....json => query_user_not_compound_v13.json} | 4 +- ..._v12.json => query_user_not_leaf_v13.json} | 4 +- ...t_v12.json => query_user_now_set_v13.json} | 4 +- tests/test_after_position.py | 188 +++++++++++ tests/test_ir_vectors_contract.py | 17 +- tests/test_mutation_pagination_guard.py | 1 + tests/test_order_by_nulls_wire.py | 10 +- tests/test_query_builder.py | 21 +- tests/test_query_immutability.py | 11 + tests/test_query_wire_vectors.py | 69 ++-- 40 files changed, 1012 insertions(+), 250 deletions(-) create mode 100644 docs/solutions/patterns/position-paging-after.md rename tests/fixtures/ir_vectors/{query_account_exists_v12.json => query_account_exists_v13.json} (92%) rename tests/fixtures/ir_vectors/{query_account_scoped_exists_v12.json => query_account_scoped_exists_v13.json} (95%) rename tests/fixtures/ir_vectors/{query_card_nulls_v12.json => query_card_nulls_v13.json} (93%) rename tests/fixtures/ir_vectors/{query_owner_nested_exists_v12.json => query_owner_nested_exists_v13.json} (93%) rename tests/fixtures/ir_vectors/{query_owner_not_exists_v12.json => query_owner_not_exists_v13.json} (91%) rename tests/fixtures/ir_vectors/{query_transaction_aggregate_v12.json => query_transaction_aggregate_v13.json} (95%) rename tests/fixtures/ir_vectors/{query_transaction_global_aggregate_v12.json => query_transaction_global_aggregate_v13.json} (95%) rename tests/fixtures/ir_vectors/{query_transaction_include_v12.json => query_transaction_include_v13.json} (92%) rename tests/fixtures/ir_vectors/{query_transaction_left_join_v12.json => query_transaction_left_join_v13.json} (94%) rename tests/fixtures/ir_vectors/{query_transaction_record_v12.json => query_transaction_record_v13.json} (92%) rename tests/fixtures/ir_vectors/{query_transaction_traversal_v12.json => query_transaction_traversal_v13.json} (96%) rename tests/fixtures/ir_vectors/{query_transaction_traversed_record_v12.json => query_transaction_traversed_record_v13.json} (95%) rename tests/fixtures/ir_vectors/{query_user_add_columns_set_v12.json => query_user_add_columns_set_v13.json} (91%) rename tests/fixtures/ir_vectors/{query_user_add_literal_set_v12.json => query_user_add_literal_set_v13.json} (92%) create mode 100644 tests/fixtures/ir_vectors/query_user_after_v13.json rename tests/fixtures/ir_vectors/{query_user_compound_v12.json => query_user_compound_v13.json} (95%) rename tests/fixtures/ir_vectors/{query_user_literal_set_v12.json => query_user_literal_set_v13.json} (94%) rename tests/fixtures/ir_vectors/{query_user_m2m_exists_v12.json => query_user_m2m_exists_v13.json} (94%) rename tests/fixtures/ir_vectors/{query_user_merge_set_v12.json => query_user_merge_set_v13.json} (93%) rename tests/fixtures/ir_vectors/{query_user_mixed_set_v12.json => query_user_mixed_set_v13.json} (93%) rename tests/fixtures/ir_vectors/{query_user_not_compound_v12.json => query_user_not_compound_v13.json} (93%) rename tests/fixtures/ir_vectors/{query_user_not_leaf_v12.json => query_user_not_leaf_v13.json} (92%) rename tests/fixtures/ir_vectors/{query_user_now_set_v12.json => query_user_now_set_v13.json} (90%) create mode 100644 tests/test_after_position.py diff --git a/crates/ferro-schema-ir/src/lib.rs b/crates/ferro-schema-ir/src/lib.rs index b023ab0..0035f44 100644 --- a/crates/ferro-schema-ir/src/lib.rs +++ b/crates/ferro-schema-ir/src/lib.rs @@ -225,8 +225,8 @@ pub enum CheckOperand { /// Query IR: filter, sort, pagination, joins, materialization plan, and /// optional M2M join context. /// -/// `ir_version: 12` (unconditional, no earlier version emitted anywhere — -/// #269, #278, #285, #292, #310, #314, #376, #377, #378, #379, #392). `set` is required +/// `ir_version: 13` (unconditional, no earlier version emitted anywhere — +/// #269, #278, #285, #292, #310, #314, #376, #377, #378, #379, #392, #393). `set` is required /// and always present (`[]` outside updates); `joins` is required and always /// present (`[]` when the query traverses no relation); every leaf and /// `order_by` entry carries a `path` (required, `[]` = root model); @@ -238,7 +238,8 @@ pub enum CheckOperand { /// the canonical SET assignment section with literal value expressions; /// v9 adds the column-ref SET value-expression kind; v10 adds binary /// `+` / `-` and the `now` clock; v11 adds Postgres ``merge``; v12 requires -/// explicit ``nulls`` on every ``order_by`` term (#392). +/// explicit ``nulls`` on every ``order_by`` term (#392); v13 adds an optional +/// ``after`` position bound on fetch payloads (#393). #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct QueryIrPayload { /// Model class name the query targets. @@ -266,6 +267,11 @@ pub struct QueryIrPayload { skip_serializing_if = "Option::is_none" )] pub offset: Option>, + /// Exclusive position bound (`after`). Omitted when unset — fetch payloads + /// carry it only when paging from a position; count and mutating payloads + /// omit it (v13, #393). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub after: Option>, /// Many-to-many join metadata JSON, deserialized into [`M2mContext`] downstream. pub m2m: Option, /// Relation joins collected from traversal (`[]` until #270 renders them). @@ -692,7 +698,7 @@ mod tests { #[test] fn query_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_compound_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_compound_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query fixture must parse"); let ir = parsed @@ -713,7 +719,7 @@ mod tests { #[test] fn query_literal_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_literal_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_literal_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query literal-set fixture must parse"); let ir = parsed @@ -722,7 +728,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query literal-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); assert_eq!(envelope.payload.set_assignments.len(), 3); assert_eq!(envelope.payload.set_assignments[0].column, "active"); match &envelope.payload.set_assignments[2].value { @@ -739,7 +745,7 @@ mod tests { #[test] fn query_mixed_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_mixed_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_mixed_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query mixed-set fixture must parse"); let ir = parsed @@ -748,7 +754,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query mixed-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); assert_eq!(envelope.payload.set_assignments.len(), 2); assert_eq!(envelope.payload.set_assignments[0].column, "email"); match &envelope.payload.set_assignments[0].value { @@ -770,7 +776,7 @@ mod tests { #[test] fn query_add_literal_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_add_literal_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_add_literal_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query add-literal-set fixture must parse"); let ir = parsed @@ -779,7 +785,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query add-literal-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Add { left, right } => { match left.as_ref() { @@ -807,7 +813,7 @@ mod tests { #[test] fn query_add_columns_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_add_columns_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_add_columns_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query add-columns-set fixture must parse"); let ir = parsed @@ -816,7 +822,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query add-columns-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Add { left, right } => { match left.as_ref() { @@ -841,7 +847,7 @@ mod tests { #[test] fn query_now_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_now_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_now_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query now-set fixture must parse"); let ir = parsed @@ -850,7 +856,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query now-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Now => {} other => panic!("expected now assignment, got {other:?}"), @@ -862,7 +868,7 @@ mod tests { #[test] fn query_merge_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_merge_set_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_merge_set_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query merge-set fixture must parse"); let ir = parsed @@ -871,7 +877,7 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query merge-set IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Merge { left, right } => { match left.as_ref() { @@ -899,7 +905,7 @@ mod tests { // leaf `IN` comparison (the NOT IN spelling) — and must survive a // deserialize/serialize round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_not_leaf_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_not_leaf_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-leaf fixture must parse"); let ir = parsed @@ -926,7 +932,7 @@ mod tests { // OR compound whole — no De Morgan expansion on the wire — and must // survive a deserialize/serialize round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_not_compound_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_not_compound_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-compound fixture must parse"); let ir = parsed @@ -954,7 +960,7 @@ mod tests { // condition tree, and must survive a deserialize/serialize // round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_account_exists_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_account_exists_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query exists fixture must parse"); let ir = parsed @@ -986,7 +992,7 @@ mod tests { // node — the exists node carries no negation flag (ADR-0008 // composition; #314). let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_owner_not_exists_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_owner_not_exists_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-exists fixture must parse"); let ir = parsed @@ -1013,7 +1019,7 @@ mod tests { // ride the exists node's own `joins` section (rendered INSIDE the // subquery). Must survive a deserialize/serialize round-trip. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_account_scoped_exists_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_account_scoped_exists_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query scoped-exists fixture must parse"); let ir = parsed @@ -1047,7 +1053,7 @@ mod tests { // mechanism, and the bare inner node omits `joins` entirely (absent, // not empty — pinned wire bytes via skip_serializing_if). let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_owner_nested_exists_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_owner_nested_exists_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query nested-exists fixture must parse"); let ir = parsed @@ -1078,7 +1084,7 @@ mod tests { // the target — with the scoped inner tree over the target model. // Must survive a deserialize/serialize round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_m2m_exists_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_m2m_exists_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query m2m-exists fixture must parse"); let ir = parsed @@ -1109,7 +1115,7 @@ mod tests { // Multi-hop `joins` section + path-carrying leaves must survive a // deserialize/serialize round-trip without drift (#270 wire stability). let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_transaction_traversal_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_traversal_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query traversal fixture must parse"); let ir = parsed @@ -1137,7 +1143,7 @@ mod tests { // deserialize/serialize round-trip without drift, and the join_type // tokens must reach Rust exactly as written on the wire. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_transaction_left_join_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_left_join_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query left_join fixture must parse"); let ir = parsed @@ -1163,7 +1169,7 @@ mod tests { // payload — predicate, order, limit, and a two-field record plan — // survives a deserialize/serialize round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_transaction_record_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_record_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query record fixture must parse"); let ir = parsed @@ -1362,7 +1368,7 @@ mod tests { // identity) — survives a deserialize/serialize round-trip without // drift. No group keys: the whole result collapses to one record. let fixture = include_str!( - "../../../tests/fixtures/ir_vectors/query_transaction_global_aggregate_v12.json" + "../../../tests/fixtures/ir_vectors/query_transaction_global_aggregate_v13.json" ); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("global aggregate fixture must parse"); @@ -1399,7 +1405,7 @@ mod tests { // joins section included — survives a deserialize/serialize // round-trip without drift. let fixture = include_str!( - "../../../tests/fixtures/ir_vectors/query_transaction_traversed_record_v12.json" + "../../../tests/fixtures/ir_vectors/query_transaction_traversed_record_v13.json" ); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query traversed record fixture must parse"); @@ -1440,7 +1446,7 @@ mod tests { // a deserialize/serialize round-trip without drift. GROUP BY does not // travel: the renderer derives it from the non-expr fields (ADR-0009). let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_transaction_aggregate_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_aggregate_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query aggregate fixture must parse"); let ir = parsed @@ -1507,7 +1513,7 @@ mod tests { // payload — root predicate, empty `joins`, and a one-path instances // plan — survives a deserialize/serialize round-trip without drift. let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_transaction_include_v12.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_include_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query include fixture must parse"); let ir = parsed @@ -1611,7 +1617,7 @@ mod tests { #[test] fn query_card_nulls_fixture_roundtrip() { // #392: golden vector with explicit nulls on every order_by term. - let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_card_nulls_v12.json"); + let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_card_nulls_v13.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query card-nulls fixture must parse"); let ir = parsed @@ -1620,18 +1626,43 @@ mod tests { .expect("fixture must contain ir envelope"); let envelope: IrEnvelope = serde_json::from_value(ir.clone()).expect("query card-nulls IR must deserialize"); - assert_eq!(envelope.ir_version, 12); + assert_eq!(envelope.ir_version, 13); assert_eq!(envelope.payload.order_by.len(), 2); assert_eq!(envelope.payload.order_by[0].nulls, "last"); assert_eq!( - envelope.payload.order_by[1].nulls, - "last", + envelope.payload.order_by[1].nulls, "last", "second term must carry explicit nulls" ); let encoded = serde_json::to_value(&envelope).expect("query card-nulls IR must serialize"); assert_eq!(encoded, ir, "query card-nulls round-trip must not drift"); } + #[test] + fn query_after_fixture_roundtrip() { + let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_user_after_v13.json"); + let parsed: serde_json::Value = + serde_json::from_str(fixture).expect("query after fixture must parse"); + let ir = parsed + .get("ir") + .cloned() + .expect("fixture must contain ir envelope"); + let envelope: IrEnvelope = + serde_json::from_value(ir.clone()).expect("query after IR must deserialize"); + assert_eq!(envelope.ir_version, 13); + let after = envelope + .payload + .after + .as_ref() + .expect("fetch payload must carry after"); + assert_eq!(after.len(), 2); + assert_eq!(after[0].kind, "int"); + assert_eq!(after[0].value, serde_json::json!(10)); + assert_eq!(after[1].kind, "int"); + assert_eq!(after[1].value, serde_json::json!(3)); + let encoded = serde_json::to_value(&envelope).expect("query after IR must serialize"); + assert_eq!(encoded, ir, "query after round-trip must not drift"); + } + #[test] fn codec_fixture_roundtrip() { let fixture = diff --git a/docs/examples/predicates.py b/docs/examples/predicates.py index fe513b6..f6c2603 100644 --- a/docs/examples/predicates.py +++ b/docs/examples/predicates.py @@ -118,6 +118,26 @@ async def main() -> None: assert oldest_first[0].name == "carol" assert len(second_page) == 2 + # --8<-- [start:after-paging] + page = ( + await User.select() + .order_by(lambda user: user.age) + .order_by(lambda user: user.id) + .limit(2) + .all() + ) + next_page = ( + await User.select() + .order_by(lambda user: user.age) + .order_by(lambda user: user.id) + .after(page[-1]) + .limit(2) + .all() + ) + # --8<-- [end:after-paging] + assert [user.name for user in page] == ["dave", "bob"] + assert [user.name for user in next_page] == ["alice", "carol"] + t0 = datetime(2026, 1, 1, tzinfo=UTC) t1 = datetime(2026, 2, 1, tzinfo=UTC) t2 = datetime(2026, 3, 1, tzinfo=UTC) diff --git a/docs/pages/guide/queries.md b/docs/pages/guide/queries.md index 7d58c44..8114cfc 100644 --- a/docs/pages/guide/queries.md +++ b/docs/pages/guide/queries.md @@ -196,7 +196,15 @@ Omitting `nulls=` on a nullable sort key means `NULLS LAST` on every backend. Pa --8<-- "docs/examples/predicates.py:ordering-slicing" ``` -Chain `.order_by()` multiple times for multi-column sorts. For robust pagination patterns, see [Pagination](../howto/pagination.md). +Chain `.order_by()` multiple times for multi-column sorts. + +To page forward from a known row, pass that row's place in the declared order to `.after()`. The bound is exclusive, the order keys must include the primary key, and every key must be a non-nullable root column: + +```python +--8<-- "docs/examples/predicates.py:after-paging" +``` + +`after(row)` is the same as `after(position_of(row))`. `after()` cannot be combined with `offset()` — a query has one start. For robust pagination patterns, see [Pagination](../howto/pagination.md). ## Executing Queries diff --git a/docs/solutions/patterns/position-paging-after.md b/docs/solutions/patterns/position-paging-after.md new file mode 100644 index 0000000..1f13251 --- /dev/null +++ b/docs/solutions/patterns/position-paging-after.md @@ -0,0 +1,23 @@ +--- +title: Exclusive stepwise compare is the after() expansion +type: pattern +tags: [query, paging, ir] +related_files: + - src/query.rs + - src/ferro/query/builder.py + - src/ferro/query/wire.py +related_issues: [393, 394, 395] +captured: 2026-08-30 +--- + +## Problem + +`after(position)` must render as an exclusive keyset bound — `(a > :a) OR (a = :a AND b > :b)` with DESC flipping `>` to `<` — without turning paging into a `where()` predicate and without copying that tree into every SELECT walker. #394 will add NULL-bucket expansion to the same bound. + +## Takeaway + +One function owns the compare tree: `exclusive_stepwise_compare` in `src/query.rs`. The SELECT walker qualifies columns, binds typed values, and ANDs the result onto WHERE. Do not sprinkle inequalities in `operations.rs`. #394 extends this function; it does not add a second expander. + +Python validates the wedge at `after()` / `position_of()` (root columns, PK included, non-nullable) and `compile_query` is the only assembler that puts `after` on the fetch payload as typed `kind`/`value` nodes. Count omits the key; mutations reject it. + +Datetime slots go through `_serialize_query_value` → pydantic JSON mode (`…Z` for UTC), the same bytes `save()` writes. `datetime.isoformat()` emits `…+00:00`; on SQLite that is a different TEXT value, so the prefix-equality arm of the stepwise compare never matches. diff --git a/src/ferro/query/builder.py b/src/ferro/query/builder.py index cfce7de..579b607 100644 --- a/src/ferro/query/builder.py +++ b/src/ferro/query/builder.py @@ -555,6 +555,7 @@ def __init__( self.order_by_clause: list[OrderByEntry] = [] self._limit: int | None = None self._offset: int | None = None + self._after: tuple[Any, ...] | None = None self._m2m_context: M2mContext | None = None # Relation paths that must render a join, insertion-ordered (full path # tuple -> registered join_type). Populated by where()/order_by() @@ -1029,15 +1030,128 @@ def offset(self, value: int) -> Self: Returns: A new ``Query`` with the clause added; ``self`` is unchanged. + Raises: + ValueError: If ``after()`` is already set — a query has one start. + Examples: >>> query = User.select().offset(20) >>> query._offset 20 """ + if self._after is not None: + raise ValueError( + "after() cannot be combined with offset(): a query has one " + "start (an offset or a position bound, never both)." + ) new = self._clone() new._offset = value return new + def _assert_after_order_keys(self) -> None: + """Require root, non-null order keys that include the primary key (#393).""" + pk = getattr(self.model_cls, "__ferro_pk__", None) + if pk is None: + raise ValueError( + f"{self.model_cls.__name__} has no primary-key column, and " + "after()/position_of() require one." + ) + if not self.order_by_clause: + raise ValueError( + "after() requires order_by() keys that include the model's " + "primary key" + ) + specs = getattr(self.model_cls, "__ferro_columns__", {}) + pk_seen = False + for entry in self.order_by_clause: + if entry.path: + dotted = ".".join((*entry.path, entry.column)) + raise ValueError( + "after() requires root-column order keys; traversed " + f"order key {dotted!r} is not supported yet" + ) + spec = specs.get(entry.column) + if spec is not None and spec.nullable: + raise ValueError( + f"after() does not support nullable order key " + f"{entry.column!r}" + ) + if entry.column == pk: + pk_seen = True + if not pk_seen: + raise ValueError( + "after() requires the model's primary key in the order keys; " + "Ferro will not append it silently" + ) + + def position_of(self, row: T) -> tuple[Any, ...]: + """Read this query's order-key tuple off a model instance. + + Args: + row: A hydrated instance of the queried model. + + Returns: + The ordered tuple of order-key values, matching ``order_by`` + declaration order. + + Raises: + TypeError: If ``row`` is not an instance of the queried model. + ValueError: If the order keys are not a legal ``after()`` set + (no primary key, a nullable key, or a traversed key). + """ + if not isinstance(row, self.model_cls): + raise TypeError( + "position_of() expected an instance of " + f"{self.model_cls.__name__}, got {type(row).__name__}" + ) + self._assert_after_order_keys() + return tuple(getattr(row, entry.column) for entry in self.order_by_clause) + + def after(self, position: tuple[Any, ...] | T) -> Self: + """Start the page after an exclusive position in the declared order. + + ``position`` is the ordered tuple of this query's order-key values, or + a model instance (sugar for ``after(position_of(row))``). Order keys + must be root columns, include the primary key, and be non-nullable. + + Args: + position: A tuple of order-key values, or a model instance. + + Returns: + A new ``Query`` with the bound set; ``self`` is unchanged. + + Raises: + TypeError: If ``position`` is neither a tuple nor a model instance. + ValueError: If the order keys are illegal for ``after()``, the + tuple has the wrong arity, a slot is ``None``, or ``offset()`` + is already set. + """ + if self._offset is not None: + raise ValueError( + "after() cannot be combined with offset(): a query has one " + "start (an offset or a position bound, never both)." + ) + if isinstance(position, tuple): + self._assert_after_order_keys() + expected = len(self.order_by_clause) + if len(position) != expected: + raise ValueError( + f"after() expected {expected} position values " + f"(one per order_by key), got {len(position)}" + ) + bound = position + elif isinstance(position, self.model_cls): + bound = self.position_of(position) + else: + raise TypeError( + "after() expected a position tuple or a model instance, " + f"got {type(position).__name__}" + ) + if any(value is None for value in bound): + raise ValueError("after() does not support None in a position slot") + new = self._clone() + new._after = bound + return new + async def all(self) -> list[T]: """Return all model instances that match the current query diff --git a/src/ferro/query/nodes.py b/src/ferro/query/nodes.py index 9ea39bc..520472e 100644 --- a/src/ferro/query/nodes.py +++ b/src/ferro/query/nodes.py @@ -1,12 +1,16 @@ """Define query AST nodes and field proxies for fluent filtering""" import difflib +import json import uuid from collections.abc import Callable, Mapping from dataclasses import dataclass +from datetime import datetime from decimal import Decimal from typing import TYPE_CHECKING, Any, Generic, NoReturn, TypeAlias, TypeVar, get_origin +from pydantic_core import to_json + TField = TypeVar("TField") TModel = TypeVar("TModel") @@ -254,7 +258,15 @@ def __repr__(self): def _serialize_query_value(value: Any) -> Any: - """Normalize Python values into JSON-friendly query payloads.""" + """Normalize Python values into JSON-friendly query payloads. + + Datetimes use pydantic JSON mode (the same canonical form as + ``save_bind_payload``): UTC is ``...Z``, not ``datetime.isoformat()``'s + ``...+00:00``. SQLite stores INSERT text in that form; ``after()`` prefix + equality is a TEXT compare there and must match the stored bytes. + """ + if isinstance(value, datetime): + return json.loads(to_json(value)) if hasattr(value, "isoformat"): return value.isoformat() if isinstance(value, (Decimal, uuid.UUID)): diff --git a/src/ferro/query/wire.py b/src/ferro/query/wire.py index 4a58ade..b1191d5 100644 --- a/src/ferro/query/wire.py +++ b/src/ferro/query/wire.py @@ -10,11 +10,12 @@ serde round-trips in the Rust crate's tests). Friend contract: :func:`compile_query` reads the query object's build state -(``where_clause``, ``order_by_clause``, ``_limit``, ``_offset``, ``_joins``, -``_explicit_edges``, ``_includes``, ``_m2m_context``, ``_projection``) -directly — the builder and this module are one package, and the seam is -``compile_query``, not a snapshot type. The builder owns chainer-time -validation and state; this module owns everything whose output is wire shape. +(``where_clause``, ``order_by_clause``, ``_limit``, ``_offset``, ``_after``, +``_joins``, ``_explicit_edges``, ``_includes``, ``_m2m_context``, +``_projection``) directly — the builder and this module are one package, and +the seam is ``compile_query``, not a snapshot type. The builder owns +chainer-time validation and state; this module owns everything whose output +is wire shape. """ from __future__ import annotations @@ -50,10 +51,10 @@ # mutate arm; they stay distinct values so guardrail errors name the caller. QueryVerb = Literal["fetch", "count", "update", "delete"] -# Always 12 (#392 — unconditional bump). v12 requires explicit ``nulls`` -# on every ``order_by`` term (omitted in Python means ``last``). Python and -# Rust ship in one wheel, so a single supported version is the whole contract. -_IR_VERSION = 12 +# Always 13 (#393 — unconditional bump). v13 adds an optional ``after`` +# position bound on fetch payloads. Python and Rust ship in one wheel, so a +# single supported version is the whole contract. +_IR_VERSION = 13 class _AbsentType: @@ -341,7 +342,8 @@ class QueryIrPayload: Mirrors ``ferro_schema_ir::QueryIrPayload`` field-for-field. ``where`` holds :meth:`QueryNode.to_ir_dict` output — the node module owns the predicate wire shape. ``limit``/``offset`` may be :data:`_ABSENT` - (mutating payloads omit the keys entirely). + (mutating payloads omit the keys entirely). ``after`` is omitted when + unset (fetch, count, and mutating payloads alike). """ model_name: str @@ -350,6 +352,7 @@ class QueryIrPayload: order_by: tuple[OrderByEntry, ...] limit: int | None | _AbsentType offset: int | None | _AbsentType + after: tuple[QueryValue, ...] | None m2m: M2mContext | None joins: tuple[QueryJoin, ...] materialization: Materialization @@ -365,6 +368,8 @@ def to_ir_dict(self) -> dict[str, Any]: payload["limit"] = self.limit if not isinstance(self.offset, _AbsentType): payload["offset"] = self.offset + if self.after: + payload["after"] = [value.to_ir_dict() for value in self.after] payload["m2m"] = self.m2m.to_ir_dict() if self.m2m is not None else None payload["joins"] = [join.to_ir_dict() for join in self.joins] payload["materialization"] = self.materialization.to_ir_dict() @@ -796,23 +801,50 @@ def _recipe_set( return tuple(compiled) +def _after_query_values(position: tuple[Any, ...]) -> tuple[QueryValue, ...]: + """Typed query-value nodes for one ``after`` bound (same shape as WHERE leaves).""" + values: list[QueryValue] = [] + for item in position: + serialized = _serialize_query_value(item) + values.append(QueryValue(kind=_query_value_kind(serialized), value=serialized)) + return tuple(values) + + +def _compile_after(query: "Query[Any]") -> tuple[QueryValue, ...] | None: + """Serialize a fetch payload's ``after`` bound, re-checking current order keys.""" + if query._after is None: + return None + query._assert_after_order_keys() + if len(query._after) != len(query.order_by_clause): + raise ValueError( + f"after() expected {len(query.order_by_clause)} position values " + f"(one per order_by key), got {len(query._after)}" + ) + return _after_query_values(query._after) + + def _reject_mutation_misuse(query: "Query[Any]", operation: str) -> None: """Reject query state a mutating verb cannot carry (FF-A A1, #273, #287). Raises: - ValueError: If ``limit()``/``offset()`` was set, the query traverses - a relation (a joined/explicit-edge path, or a where-clause leaf - carrying a non-empty path — portable SQL has no ``UPDATE/DELETE - ... JOIN``; a join-free shadow-FK filter stays allowed), or the - query carries ``include()``. Rejected here, before any DB - round-trip (the Rust guard from #270 stays as boundary defense). + ValueError: If ``limit()``/``offset()``/``after()`` was set, the query + traverses a relation (a joined/explicit-edge path, or a where-clause + leaf carrying a non-empty path — portable SQL has no + ``UPDATE/DELETE ... JOIN``; a join-free shadow-FK filter stays + allowed), or the query carries ``include()``. Rejected here, before + any DB round-trip (the Rust guard from #270 stays as boundary + defense). """ - if query._limit is not None or query._offset is not None: + if ( + query._limit is not None + or query._offset is not None + or query._after is not None + ): raise ValueError( - f"{operation}() does not support limit/offset: portable SQL has " - f"no {operation.upper()} ... LIMIT. Remove the .limit()/.offset() " - f"call, or fetch primary keys first and {operation} by " - "primary-key set." + f"{operation}() does not support limit/offset/after: portable SQL " + f"has no {operation.upper()} ... LIMIT. Remove the " + f".limit()/.offset()/.after() call, or fetch primary keys first " + f"and {operation} by primary-key set." ) if ( query._joins @@ -890,12 +922,14 @@ def compile_query( What each verb carries is policy, stated here once: - - ``fetch`` carries everything: ordering, paging, m2m context, joins, - and the query's own materialization plan. + - ``fetch`` carries everything: ordering, paging (``limit``/``offset`` + keys, plus an optional ``after`` bound), m2m context, joins, and the + query's own materialization plan. - ``count`` is unaffected by ordering, paging, and projection (PRD #277 verb table): it materializes a scalar, so ordering is dropped, paging - is ``null``, and the plan is ``root_instances`` even on a projected - query — joins and the m2m context still shape membership. + is ``null`` (``after`` omitted), and the plan is ``root_instances`` + even on a projected query — joins and the m2m context still shape + membership. - ``update``/``delete`` are single-table write shapes: guardrails reject paging, traversal, and includes (:func:`_reject_mutation_misuse`), and the payload omits the paging keys entirely. @@ -932,6 +966,7 @@ def compile_query( order_by=(), limit=_ABSENT, offset=_ABSENT, + after=None, m2m=None, joins=(), materialization=RootInstances(), @@ -944,6 +979,7 @@ def compile_query( order_by=(), limit=None, offset=None, + after=None, m2m=query._m2m_context, joins=_serialize_joins(query), materialization=RootInstances(), @@ -956,6 +992,7 @@ def compile_query( order_by=tuple(query.order_by_clause), limit=query._limit, offset=query._offset, + after=_compile_after(query), m2m=query._m2m_context, joins=_serialize_joins(query), materialization=_materialization(query), diff --git a/src/operations.rs b/src/operations.rs index b68b4f5..e72f2a4 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -252,12 +252,12 @@ fn tx_remove(session_id: Option<&str>, tx_id: &str) -> PyResult( Ok(instance) } -/// Enforce the v8 verb contract for `limit`/`offset` key presence. +/// Enforce the v13 verb contract for paging-key presence (#393). fn validate_paging_shape(plan: &QueryPlan, operation: &str) -> PyResult<()> { match operation { - "update" | "delete" if plan.limit.is_some() || plan.offset.is_some() => { + "update" | "delete" + if plan.limit.is_some() || plan.offset.is_some() || plan.after.is_some() => + { Err(pyo3::exceptions::PyValueError::new_err(format!( - "{operation} QueryIR must omit limit/offset keys" + "{operation} QueryIR must omit limit/offset/after keys" ))) } "fetch" | "count" if plan.limit.is_none() || plan.offset.is_none() => { @@ -1231,6 +1233,14 @@ fn validate_paging_shape(plan: &QueryPlan, operation: &str) -> PyResult<()> { "{operation} QueryIR must carry limit/offset keys (null when unset)" ))) } + "count" if plan.after.is_some() => Err(pyo3::exceptions::PyValueError::new_err( + "count QueryIR must omit the after key (paging is dropped)".to_string(), + )), + "fetch" if plan.after.is_some() && plan.offset.flatten().is_some() => { + Err(pyo3::exceptions::PyValueError::new_err( + "after cannot be combined with offset: a query has one start".to_string(), + )) + } "update" | "delete" | "fetch" | "count" => Ok(()), _ => Err(pyo3::exceptions::PyValueError::new_err(format!( "unknown QueryIR operation {operation:?}" @@ -2779,12 +2789,15 @@ pub fn fetch_filtered<'py>( // WHERE columns are qualified by their relation-path alias (root leaf -> // root table, path leaf -> its JOIN alias). A path with no matching join // entry is a loud error, never a silently unqualified column. - select.cond_where(query_condition_with_joins( - &plan, - backend, - &table_name, - &join_plan, - )?); + let mut condition = + query_condition_with_joins(&plan, backend, &table_name, &join_plan)?; + if let Some(after) = plan + .after_condition(backend, &table_name, &join_plan) + .map_err(pyo3::exceptions::PyValueError::new_err)? + { + condition = condition.add(after); + } + select.cond_where(condition); // ORDER BY terms are qualified the same way as WHERE leaves: an empty // path qualifies by the root table, a relation path by its JOIN alias // (#271). A path with no matching join entry is a loud error — the @@ -5388,7 +5401,7 @@ mod mutation_pagination_guard_tests { fn envelope_without_pagination_keys() -> String { serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Widget", @@ -5576,10 +5589,10 @@ mod mutation_pagination_guard_tests { mod query_ir_version_gate_tests { use super::query_plan_from_ir_json; - fn v12_envelope() -> serde_json::Value { + fn v13_envelope() -> serde_json::Value { serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Widget", @@ -5597,7 +5610,7 @@ mod query_ir_version_gate_tests { /// Assert a rejected envelope's message names the received version, this /// build's supported version (8), and the one-wheel fix — the actionable /// shape pinned since the v1-at-v2 bump (#269), re-pinned at v3 (#278), - /// v4 (#285), v5 (#292), v6 (#310), v7 (#314), v8 (#376), v9 (#377), v10 (#378), v11 (#379), and v12 (#392). + /// v4 (#285), v5 (#292), v6 (#310), v7 (#314), v8 (#376), v9 (#377), v10 (#378), v11 (#379), v12 (#392), and v13 (#393). fn assert_actionable_version_rejection(err: pyo3::PyErr, received: char) { pyo3::Python::attach(|py| { assert!(err.is_instance_of::(py)); @@ -5608,7 +5621,7 @@ mod query_ir_version_gate_tests { "message should name the received version: {msg}" ); assert!( - msg.contains("12"), + msg.contains("13"), "message should name the supported version: {msg}" ); assert!( @@ -5622,21 +5635,21 @@ mod query_ir_version_gate_tests { } #[test] - fn accepts_version_12() { - query_plan_from_ir_json(&v12_envelope().to_string()) - .expect("a well-formed v12 envelope must be accepted"); + fn accepts_version_13() { + query_plan_from_ir_json(&v13_envelope().to_string()) + .expect("a well-formed v13 envelope must be accepted"); } #[test] fn rejects_v8_payload_without_set_section() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["payload"] .as_object_mut() .expect("payload object") .remove("set"); let err = query_plan_from_ir_json(&envelope.to_string()) - .expect_err("a v12 payload without its required SET section must be rejected"); + .expect_err("a v13 payload without its required SET section must be rejected"); assert!( err.to_string().contains("set"), "must name the missing section: {err}" @@ -5645,7 +5658,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v7_envelope_with_actionable_message() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["ir_version"] = serde_json::json!(7); envelope["payload"] .as_object_mut() @@ -5659,7 +5672,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v8_envelope_with_actionable_message() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["ir_version"] = serde_json::json!(8); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5669,7 +5682,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v9_envelope_with_actionable_message() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["ir_version"] = serde_json::json!(9); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5679,7 +5692,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v10_envelope_with_actionable_message() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["ir_version"] = serde_json::json!(10); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5690,7 +5703,7 @@ mod query_ir_version_gate_tests { "message should name the received version: {msg}" ); assert!( - msg.contains("12"), + msg.contains("13"), "message should name the supported version: {msg}" ); assert!( @@ -5701,7 +5714,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v11_envelope_with_actionable_message() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["ir_version"] = serde_json::json!(11); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5711,8 +5724,30 @@ mod query_ir_version_gate_tests { msg.contains("11"), "message should name the received version: {msg}" ); + assert!( + msg.contains("13"), + "message should name the supported version: {msg}" + ); + assert!( + msg.to_lowercase().contains("one wheel"), + "message should explain Python/Rust ship in one wheel: {msg}" + ); + } + + #[test] + fn rejects_v12_envelope_with_actionable_message() { + let mut envelope = v13_envelope(); + envelope["ir_version"] = serde_json::json!(12); + + let err = query_plan_from_ir_json(&envelope.to_string()) + .expect_err("a v12 envelope must be rejected before payload parsing"); + let msg = err.to_string(); assert!( msg.contains("12"), + "message should name the received version: {msg}" + ); + assert!( + msg.contains("13"), "message should name the supported version: {msg}" ); assert!( @@ -5904,14 +5939,14 @@ mod query_ir_version_gate_tests { #[test] fn rejects_unsupported_future_version() { - let mut envelope = v12_envelope(); - envelope["ir_version"] = serde_json::json!(13); + let mut envelope = v13_envelope(); + envelope["ir_version"] = serde_json::json!(14); let err = query_plan_from_ir_json(&envelope.to_string()) .expect_err("an unsupported future version must be rejected"); let msg = err.to_string(); assert!( - msg.contains("13"), + msg.contains("14"), "message should name the received version: {msg}" ); } @@ -5920,7 +5955,7 @@ mod query_ir_version_gate_tests { /// naming the bad kind and the supported ones (#278). #[test] fn rejects_unknown_materialization_kind() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["payload"]["materialization"] = serde_json::json!({"kind": "row_dicts"}); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5937,7 +5972,7 @@ mod query_ir_version_gate_tests { /// the plan travels with the query as data (ADR-0007), never defaulted. #[test] fn rejects_v8_payload_missing_materialization() { - let mut envelope = v12_envelope(); + let mut envelope = v13_envelope(); envelope["payload"] .as_object_mut() .unwrap() @@ -5964,7 +5999,7 @@ mod materialization_walker_gate_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Widget", @@ -6057,7 +6092,7 @@ mod record_select_list_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Transaction", @@ -6560,7 +6595,7 @@ mod instances_select_list_tests { let mut plan = query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Transaction", @@ -6720,7 +6755,7 @@ mod mutation_qualification_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Widget", @@ -6807,7 +6842,7 @@ mod select_join_render_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Transaction", @@ -6914,7 +6949,7 @@ mod select_join_render_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": model_name, @@ -6937,7 +6972,7 @@ mod select_join_render_tests { let plan = query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "set": [], "model_name": "Transaction", @@ -7088,11 +7123,7 @@ mod order_by_nulls_render_tests { use super::apply_order_by_term; use sea_query::{Alias, Expr, PostgresQueryBuilder, Query, SqliteQueryBuilder}; - fn order_term( - column: &str, - direction: &str, - nulls: &str, - ) -> ferro_schema_ir::QueryOrderBy { + fn order_term(column: &str, direction: &str, nulls: &str) -> ferro_schema_ir::QueryOrderBy { ferro_schema_ir::QueryOrderBy { column: column.to_string(), direction: direction.to_string(), diff --git a/src/query.rs b/src/query.rs index e127dd2..198b619 100644 --- a/src/query.rs +++ b/src/query.rs @@ -7,6 +7,7 @@ use crate::state::Dialect; use ferro_schema_ir::{ Materialization, QueryIrPayload, QueryJoin, QueryNode, QueryOrderBy, QuerySetAssignment, + QueryValue, }; use sea_query::{Alias, Condition, Expr, JoinType, SimpleExpr}; use serde::{Deserialize, Serialize}; @@ -101,6 +102,50 @@ pub fn qualify_column_with_joins( qualify_leaf_column(&qualifier, column, path) } +/// One term of an exclusive stepwise (keyset) compare (#393). +/// +/// #394 extends this with NULL-bucket facts; keep compare logic here, not in +/// the SELECT builder. +pub struct StepwiseKey { + /// Table-qualified order-key column. + pub column: Expr, + /// `"asc"` or `"desc"` (case-insensitive). + pub direction: String, + /// Bound value for this key, already typed as a SeaQuery expression. + pub bound: SimpleExpr, +} + +/// Exclusive stepwise compare: rows strictly after the bound in declared order. +/// +/// For two NOT NULL ASC keys: `(a > :a) OR (a = :a AND b > :b)`. DESC flips +/// `>` to `<`. Mixed directions apply per key. #394 will extend this function +/// for NULL-aware expansion — callers must not duplicate the compare tree. +pub fn exclusive_stepwise_compare(keys: &[StepwiseKey]) -> Result { + if keys.is_empty() { + return Err("exclusive stepwise compare requires at least one order key".to_string()); + } + let mut any = Condition::any(); + for i in 0..keys.len() { + let mut prefix = Condition::all(); + for key in &keys[..i] { + prefix = prefix.add(key.column.clone().eq(key.bound.clone())); + } + prefix = prefix.add(stepwise_inequality(&keys[i])?); + any = any.add(prefix); + } + Ok(any) +} + +fn stepwise_inequality(key: &StepwiseKey) -> Result { + match key.direction.to_ascii_lowercase().as_str() { + "asc" => Ok(key.column.clone().gt(key.bound.clone())), + "desc" => Ok(key.column.clone().lt(key.bound.clone())), + other => Err(format!( + "invalid order_by direction {other:?}: expected \"asc\" or \"desc\"" + )), + } +} + /// One rendered relation JOIN edge: ` AS ON /// . = .`. #[derive(Debug, Clone)] @@ -207,7 +252,8 @@ impl JoinPlanBuilder { } self.used.insert(alias.clone()); self.prefix_alias.insert(prefix.clone(), alias.clone()); - self.prefix_table.insert(prefix.clone(), hop.to_table.clone()); + self.prefix_table + .insert(prefix.clone(), hop.to_table.clone()); self.renders.push(JoinRender { join_type: edge_join_type.to_string(), to_table: hop.to_table.clone(), @@ -346,6 +392,9 @@ pub struct QueryPlan { pub limit: Option>, /// `OFFSET` key presence and value (`None` = absent, `Some(None)` = null). pub offset: Option>, + /// Exclusive position bound (`None` = omitted). Fetch only; count and + /// mutations omit it (#393). + pub after: Option>, /// Relation JOINs collected from WHERE traversal, in registration order. /// Empty for non-traversal queries; rendered by the SELECT walkers (#270). pub joins: Vec, @@ -411,6 +460,7 @@ impl QueryPlan { order_by: payload.order_by, limit: payload.limit, offset: payload.offset, + after: payload.after, joins: payload.joins, m2m, materialization: payload.materialization, @@ -429,7 +479,10 @@ impl QueryPlan { /// traversal here is misuse; the error detail names the offending shape. pub fn ensure_no_traversal(&self) -> Result<(), String> { if !self.joins.is_empty() { - return Err(format!("query carries {} relation join(s)", self.joins.len())); + return Err(format!( + "query carries {} relation join(s)", + self.joins.len() + )); } for node in &self.where_clause { reject_non_empty_leaf_path(node)?; @@ -466,11 +519,7 @@ impl QueryPlan { /// /// # Errors /// Returns `Err(String)` for an unknown `join_type` (see [`validate_join_type`]). - pub fn build_join_plan( - &self, - root_table: &str, - reserved: &[&str], - ) -> Result { + pub fn build_join_plan(&self, root_table: &str, reserved: &[&str]) -> Result { // Validate every entry's join_type and collect the set of LEFT edges. // A `"left"` entry is whole-path, so every one of its prefixes is a LEFT // edge; an edge is LEFT iff any `"left"` entry contains it. @@ -565,6 +614,44 @@ impl QueryPlan { self.build_condition(backend, &qualifier) } + /// Bind and qualify an ``after`` bound into an exclusive stepwise condition. + /// + /// Returns ``None`` when the payload omitted ``after``. Compare logic lives + /// in [`exclusive_stepwise_compare`] so #394 can extend one function. + pub fn after_condition( + &self, + backend: Dialect, + root_table: &str, + join_plan: &JoinPlan, + ) -> Result, String> { + let Some(values) = &self.after else { + return Ok(None); + }; + if self.offset.flatten().is_some() { + return Err("after cannot be combined with offset: a query has one start".to_string()); + } + if values.len() != self.order_by.len() { + return Err(format!( + "after bound has {} values but order_by has {} keys", + values.len(), + self.order_by.len() + )); + } + let mut keys = Vec::with_capacity(values.len()); + for (order, value) in self.order_by.iter().zip(values) { + let column = + qualify_column_with_joins(root_table, join_plan, &order.column, &order.path)?; + let bound = + self.value_rhs_simple_expr_for_backend(&order.column, &value.value, false, backend); + keys.push(StepwiseKey { + column, + direction: order.direction.clone(), + bound, + }); + } + Ok(Some(exclusive_stepwise_compare(&keys)?)) + } + fn build_condition( &self, backend: Dialect, @@ -854,7 +941,13 @@ impl QueryPlan { &self, qualifier: &ColumnQualifier<'_>, path: &[String], - ) -> Result<(Option<&crate::codec_plan::ModelCodecPlan>, &HashMap), String> { + ) -> Result< + ( + Option<&crate::codec_plan::ModelCodecPlan>, + &HashMap, + ), + String, + > { // Inside an EXISTS subquery (#314/#315) a leaf belongs to the // subquery scope — the scope table for an empty path, the inner // join plan's hop table for a traversed one — NEVER the root model; @@ -923,9 +1016,11 @@ impl QueryPlan { #[cfg(test)] mod tests { - use super::QueryPlan; + use super::{QueryPlan, StepwiseKey, exclusive_stepwise_compare}; use crate::state::Dialect; - use sea_query::{Alias, PostgresQueryBuilder, Query, SqliteQueryBuilder, Value as SeaValue}; + use sea_query::{ + Alias, Expr, PostgresQueryBuilder, Query, SqliteQueryBuilder, Value as SeaValue, + }; use serde_json::json; use std::collections::HashMap; @@ -941,6 +1036,7 @@ mod tests { order_by: Vec::new(), limit: Some(None), offset: Some(None), + after: None, joins: Vec::new(), m2m: None, materialization: ferro_schema_ir::Materialization::RootInstances, @@ -960,6 +1056,66 @@ mod tests { values.0.into_iter().next().expect("one value") } + fn render_stepwise(keys: &[StepwiseKey]) -> String { + let cond = exclusive_stepwise_compare(keys).expect("stepwise compare"); + let mut select = Query::select(); + select + .column(Alias::new("id")) + .from(Alias::new("t")) + .cond_where(cond); + select.to_string(SqliteQueryBuilder) + } + + #[test] + fn exclusive_stepwise_compare_asc_two_not_null_keys() { + let sql = render_stepwise(&[ + StepwiseKey { + column: Expr::col(Alias::new("a")), + direction: "asc".into(), + bound: Expr::val(1).into(), + }, + StepwiseKey { + column: Expr::col(Alias::new("b")), + direction: "asc".into(), + bound: Expr::val(2).into(), + }, + ]); + let flat = sql.replace('"', "").replace('`', ""); + assert!( + flat.contains("a > 1") && flat.contains("a = 1") && flat.contains("b > 2"), + "ASC exclusive compare: {sql}" + ); + assert!( + !flat.contains("b = 2"), + "exclusive bound must not include the all-equal term: {sql}" + ); + } + + #[test] + fn exclusive_stepwise_compare_desc_two_not_null_keys() { + let sql = render_stepwise(&[ + StepwiseKey { + column: Expr::col(Alias::new("a")), + direction: "desc".into(), + bound: Expr::val(1).into(), + }, + StepwiseKey { + column: Expr::col(Alias::new("b")), + direction: "desc".into(), + bound: Expr::val(2).into(), + }, + ]); + let flat = sql.replace('"', "").replace('`', ""); + assert!( + flat.contains("a < 1") && flat.contains("a = 1") && flat.contains("b < 2"), + "DESC exclusive compare: {sql}" + ); + assert!( + !flat.contains("b = 2"), + "exclusive bound must not include the all-equal term: {sql}" + ); + } + #[test] fn query_plan_builds_from_ir_payload_and_lowers_null_eq_to_is_null() { let payload: ferro_schema_ir::QueryIrPayload = serde_json::from_value(serde_json::json!({ @@ -1521,7 +1677,11 @@ mod tests { let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); // A 1-hop path and a 2-hop path sharing its prefix produce exactly two edges. - assert_eq!(join_plan.renders.len(), 2, "shared prefix must dedup to 2 JOINs"); + assert_eq!( + join_plan.renders.len(), + 2, + "shared prefix must dedup to 2 JOINs" + ); assert_eq!(join_plan.renders[0].alias, "j1_account"); assert_eq!(join_plan.renders[0].prev_alias, "transaction"); assert_eq!(join_plan.renders[1].alias, "j2_owner"); @@ -1551,7 +1711,8 @@ mod tests { "owner".to_string(), ), ); - plan.hop_enum_udt.insert("owner".to_string(), HashMap::new()); + plan.hop_enum_udt + .insert("owner".to_string(), HashMap::new()); let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); let sql = plan .to_condition_with_joins(Dialect::Postgres, "transaction", &join_plan) @@ -1807,9 +1968,15 @@ mod tests { // `.include(account)` + `.include(account.owner)` renders the same // two joins as `.include(account.owner)` alone (#287): shared // prefixes dedup by path identity, every include-only edge LEFT. - let expanded = union_plan(json!([]), json!([[account_hop()], [account_hop(), owner_hop()]])); + let expanded = union_plan( + json!([]), + json!([[account_hop()], [account_hop(), owner_hop()]]), + ); let collapsed = union_plan(json!([]), json!([[account_hop(), owner_hop()]])); - let reversed = union_plan(json!([]), json!([[account_hop(), owner_hop()], [account_hop()]])); + let reversed = union_plan( + json!([]), + json!([[account_hop(), owner_hop()], [account_hop()]]), + ); for plan in [expanded, collapsed, reversed] { let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); assert_eq!(join_plan.renders.len(), 2, "shared prefix dedups"); @@ -1832,8 +1999,14 @@ mod tests { ); let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); assert_eq!(join_plan.renders.len(), 2); - assert_eq!(join_plan.renders[0].join_type, "inner", "shared prefix untouched"); - assert_eq!(join_plan.renders[1].join_type, "left", "include-only edge LEFT"); + assert_eq!( + join_plan.renders[0].join_type, "inner", + "shared prefix untouched" + ); + assert_eq!( + join_plan.renders[1].join_type, "left", + "include-only edge LEFT" + ); } #[test] @@ -1932,13 +2105,8 @@ mod tests { let plan = QueryPlan::from_ir_payload(traversal_payload()).expect("plan builds"); let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); - let root_col = super::qualify_column_with_joins( - "transaction", - &join_plan, - "id", - &[], - ) - .expect("root column qualifies"); + let root_col = super::qualify_column_with_joins("transaction", &join_plan, "id", &[]) + .expect("root column qualifies"); let sql = Query::select() .expr(root_col) .to_string(PostgresQueryBuilder) @@ -2032,11 +2200,14 @@ mod tests { // nullable integer column "count" so model_column lookups succeed. crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetIntNull".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "count": {"anyOf": [{"type": "integer"}, {"type": "null"}]} - } - }), "widget".to_string()), + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "count": {"anyOf": [{"type": "integer"}, {"type": "null"}]} + } + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetIntNull"); @@ -2057,11 +2228,14 @@ mod tests { fn null_rhs_emits_typed_bool_null_for_bool_column() { crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetBoolNull".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "active": {"anyOf": [{"type": "boolean"}, {"type": "null"}]} - } - }), "widget".to_string()), + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "active": {"anyOf": [{"type": "boolean"}, {"type": "null"}]} + } + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetBoolNull"); @@ -2082,11 +2256,14 @@ mod tests { fn null_rhs_emits_typed_uuid_null_for_uuid_column() { crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetUuidNull".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "id": {"anyOf": [{"type": "string", "format": "uuid"}, {"type": "null"}]} - } - }), "widget".to_string()), + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "id": {"anyOf": [{"type": "string", "format": "uuid"}, {"type": "null"}]} + } + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetUuidNull"); @@ -2107,11 +2284,14 @@ mod tests { fn binary_rhs_emits_typed_bytes_no_cast() { crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetBinary".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "blob": {"type": "string", "format": "binary"} - } - }), "widget".to_string()), + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "blob": {"type": "string", "format": "binary"} + } + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetBinary"); @@ -2159,11 +2339,14 @@ mod tests { fn enum_rhs_skips_cast_without_native_enum_column() { crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetTextColor".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "color": {"enum_type_name": "color", "db_type": "text"} - } - }), "widget".to_string()), + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "color": {"enum_type_name": "color", "db_type": "text"} + } + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetTextColor"); @@ -2188,20 +2371,23 @@ mod tests { // Decimal still uses CAST AS numeric on Postgres. crate::state::MODEL_REGISTRY.write().unwrap().insert( "WidgetDecimal".to_string(), - crate::state::RegisteredModel::new_for_test(json!({ - "properties": { - "amount": { - // The enriched shape registration emits for Decimal - // annotations; the pattern alone must NOT make a - // column decimal (F5). - "anyOf": [ - {"type": "number"}, - {"type": "string", "pattern": "^-?\\d+(\\.\\d+)?$"} - ], - "format": "decimal" + crate::state::RegisteredModel::new_for_test( + json!({ + "properties": { + "amount": { + // The enriched shape registration emits for Decimal + // annotations; the pattern alone must NOT make a + // column decimal (F5). + "anyOf": [ + {"type": "number"}, + {"type": "string", "pattern": "^-?\\d+(\\.\\d+)?$"} + ], + "format": "decimal" + } } - } - }), "widget".to_string()), + }), + "widget".to_string(), + ), ); let plan = empty_query_plan("WidgetDecimal"); diff --git a/tests/fixtures/ir_vectors/README.md b/tests/fixtures/ir_vectors/README.md index c2b8608..3873790 100644 --- a/tests/fixtures/ir_vectors/README.md +++ b/tests/fixtures/ir_vectors/README.md @@ -28,9 +28,10 @@ Rules: - `domain` and `ir.ir_kind` must match. - `ir.ir_version` must equal `1` for `schema` and `codec` vectors. `query` - vectors are on `ir_version: 12` (#392 — explicit `nulls` on every `order_by` - term; omitted in Python means `last`; every payload carries a required - canonical `set` list). v7 introduced the recursive + vectors are on `ir_version: 13` (#393 — optional `after` position bound on + fetch payloads; omitted when unset; every `order_by` term still carries + explicit `nulls`; every payload carries a required canonical `set` list). + v7 introduced the recursive `exists` node kind beside `leaf`/`compound`/`not` (ADR-0007): `{"node_kind": "exists", "hops": [...], "where": [...]}` — `hops` is the correlation hop path in the `joins`-section hop shape (1 hop reverse FK, diff --git a/tests/fixtures/ir_vectors/query_account_exists_v12.json b/tests/fixtures/ir_vectors/query_account_exists_v13.json similarity index 92% rename from tests/fixtures/ir_vectors/query_account_exists_v12.json rename to tests/fixtures/ir_vectors/query_account_exists_v13.json index 84623b8..5f36b0d 100644 --- a/tests/fixtures/ir_vectors/query_account_exists_v12.json +++ b/tests/fixtures/ir_vectors/query_account_exists_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_account_exists_v12", + "vector_name": "query_account_exists_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Account", "where": [ diff --git a/tests/fixtures/ir_vectors/query_account_scoped_exists_v12.json b/tests/fixtures/ir_vectors/query_account_scoped_exists_v13.json similarity index 95% rename from tests/fixtures/ir_vectors/query_account_scoped_exists_v12.json rename to tests/fixtures/ir_vectors/query_account_scoped_exists_v13.json index ff3307e..cb16024 100644 --- a/tests/fixtures/ir_vectors/query_account_scoped_exists_v12.json +++ b/tests/fixtures/ir_vectors/query_account_scoped_exists_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_account_scoped_exists_v12", + "vector_name": "query_account_scoped_exists_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Account", "where": [ diff --git a/tests/fixtures/ir_vectors/query_card_nulls_v12.json b/tests/fixtures/ir_vectors/query_card_nulls_v13.json similarity index 93% rename from tests/fixtures/ir_vectors/query_card_nulls_v12.json rename to tests/fixtures/ir_vectors/query_card_nulls_v13.json index d986863..b20b003 100644 --- a/tests/fixtures/ir_vectors/query_card_nulls_v12.json +++ b/tests/fixtures/ir_vectors/query_card_nulls_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_card_nulls_v12", + "vector_name": "query_card_nulls_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Card", "where": [ diff --git a/tests/fixtures/ir_vectors/query_owner_nested_exists_v12.json b/tests/fixtures/ir_vectors/query_owner_nested_exists_v13.json similarity index 93% rename from tests/fixtures/ir_vectors/query_owner_nested_exists_v12.json rename to tests/fixtures/ir_vectors/query_owner_nested_exists_v13.json index bbc542d..b159155 100644 --- a/tests/fixtures/ir_vectors/query_owner_nested_exists_v12.json +++ b/tests/fixtures/ir_vectors/query_owner_nested_exists_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_owner_nested_exists_v12", + "vector_name": "query_owner_nested_exists_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Owner", "where": [ diff --git a/tests/fixtures/ir_vectors/query_owner_not_exists_v12.json b/tests/fixtures/ir_vectors/query_owner_not_exists_v13.json similarity index 91% rename from tests/fixtures/ir_vectors/query_owner_not_exists_v12.json rename to tests/fixtures/ir_vectors/query_owner_not_exists_v13.json index fcdb555..4ce8383 100644 --- a/tests/fixtures/ir_vectors/query_owner_not_exists_v12.json +++ b/tests/fixtures/ir_vectors/query_owner_not_exists_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_owner_not_exists_v12", + "vector_name": "query_owner_not_exists_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Owner", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_aggregate_v12.json b/tests/fixtures/ir_vectors/query_transaction_aggregate_v13.json similarity index 95% rename from tests/fixtures/ir_vectors/query_transaction_aggregate_v12.json rename to tests/fixtures/ir_vectors/query_transaction_aggregate_v13.json index f6c11e3..0a8913b 100644 --- a/tests/fixtures/ir_vectors/query_transaction_aggregate_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_aggregate_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_aggregate_v12", + "vector_name": "query_transaction_aggregate_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v12.json b/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v13.json similarity index 95% rename from tests/fixtures/ir_vectors/query_transaction_global_aggregate_v12.json rename to tests/fixtures/ir_vectors/query_transaction_global_aggregate_v13.json index 4cca4e9..dbb9c5b 100644 --- a/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_global_aggregate_v12", + "vector_name": "query_transaction_global_aggregate_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_include_v12.json b/tests/fixtures/ir_vectors/query_transaction_include_v13.json similarity index 92% rename from tests/fixtures/ir_vectors/query_transaction_include_v12.json rename to tests/fixtures/ir_vectors/query_transaction_include_v13.json index 2092025..9b999e8 100644 --- a/tests/fixtures/ir_vectors/query_transaction_include_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_include_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_include_v12", + "vector_name": "query_transaction_include_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_left_join_v12.json b/tests/fixtures/ir_vectors/query_transaction_left_join_v13.json similarity index 94% rename from tests/fixtures/ir_vectors/query_transaction_left_join_v12.json rename to tests/fixtures/ir_vectors/query_transaction_left_join_v13.json index 8bb74b9..d6e82af 100644 --- a/tests/fixtures/ir_vectors/query_transaction_left_join_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_left_join_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_left_join_v12", + "vector_name": "query_transaction_left_join_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_record_v12.json b/tests/fixtures/ir_vectors/query_transaction_record_v13.json similarity index 92% rename from tests/fixtures/ir_vectors/query_transaction_record_v12.json rename to tests/fixtures/ir_vectors/query_transaction_record_v13.json index fcd19c6..4edcf8a 100644 --- a/tests/fixtures/ir_vectors/query_transaction_record_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_record_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_record_v12", + "vector_name": "query_transaction_record_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_traversal_v12.json b/tests/fixtures/ir_vectors/query_transaction_traversal_v13.json similarity index 96% rename from tests/fixtures/ir_vectors/query_transaction_traversal_v12.json rename to tests/fixtures/ir_vectors/query_transaction_traversal_v13.json index c8a7774..8a25541 100644 --- a/tests/fixtures/ir_vectors/query_transaction_traversal_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_traversal_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_traversal_v12", + "vector_name": "query_transaction_traversal_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_traversed_record_v12.json b/tests/fixtures/ir_vectors/query_transaction_traversed_record_v13.json similarity index 95% rename from tests/fixtures/ir_vectors/query_transaction_traversed_record_v12.json rename to tests/fixtures/ir_vectors/query_transaction_traversed_record_v13.json index 6ff560b..8bc1c29 100644 --- a/tests/fixtures/ir_vectors/query_transaction_traversed_record_v12.json +++ b/tests/fixtures/ir_vectors/query_transaction_traversed_record_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_traversed_record_v12", + "vector_name": "query_transaction_traversed_record_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_add_columns_set_v12.json b/tests/fixtures/ir_vectors/query_user_add_columns_set_v13.json similarity index 91% rename from tests/fixtures/ir_vectors/query_user_add_columns_set_v12.json rename to tests/fixtures/ir_vectors/query_user_add_columns_set_v13.json index 7ab8900..3ec606a 100644 --- a/tests/fixtures/ir_vectors/query_user_add_columns_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_add_columns_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_add_columns_set_v12", + "vector_name": "query_user_add_columns_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_add_literal_set_v12.json b/tests/fixtures/ir_vectors/query_user_add_literal_set_v13.json similarity index 92% rename from tests/fixtures/ir_vectors/query_user_add_literal_set_v12.json rename to tests/fixtures/ir_vectors/query_user_add_literal_set_v13.json index 401d51d..b15de28 100644 --- a/tests/fixtures/ir_vectors/query_user_add_literal_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_add_literal_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_add_literal_set_v12", + "vector_name": "query_user_add_literal_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_after_v13.json b/tests/fixtures/ir_vectors/query_user_after_v13.json new file mode 100644 index 0000000..dbfbb03 --- /dev/null +++ b/tests/fixtures/ir_vectors/query_user_after_v13.json @@ -0,0 +1,56 @@ +{ + "vector_name": "query_user_after_v13", + "domain": "query", + "expect_valid": true, + "ir": { + "ir_kind": "query", + "ir_version": 13, + "payload": { + "model_name": "User", + "where": [ + { + "node_kind": "leaf", + "column": "active", + "operator": "==", + "value": { + "kind": "bool", + "value": true + }, + "path": [] + } + ], + "set": [], + "order_by": [ + { + "column": "score", + "direction": "asc", + "path": [], + "nulls": "last" + }, + { + "column": "id", + "direction": "asc", + "path": [], + "nulls": "last" + } + ], + "limit": 5, + "offset": null, + "after": [ + { + "kind": "int", + "value": 10 + }, + { + "kind": "int", + "value": 3 + } + ], + "m2m": null, + "joins": [], + "materialization": { + "kind": "root_instances" + } + } + } +} diff --git a/tests/fixtures/ir_vectors/query_user_compound_v12.json b/tests/fixtures/ir_vectors/query_user_compound_v13.json similarity index 95% rename from tests/fixtures/ir_vectors/query_user_compound_v12.json rename to tests/fixtures/ir_vectors/query_user_compound_v13.json index 80b84a4..9e703f3 100644 --- a/tests/fixtures/ir_vectors/query_user_compound_v12.json +++ b/tests/fixtures/ir_vectors/query_user_compound_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_compound_v12", + "vector_name": "query_user_compound_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_literal_set_v12.json b/tests/fixtures/ir_vectors/query_user_literal_set_v13.json similarity index 94% rename from tests/fixtures/ir_vectors/query_user_literal_set_v12.json rename to tests/fixtures/ir_vectors/query_user_literal_set_v13.json index 7f8709c..2bec0ef 100644 --- a/tests/fixtures/ir_vectors/query_user_literal_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_literal_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_literal_set_v12", + "vector_name": "query_user_literal_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_m2m_exists_v12.json b/tests/fixtures/ir_vectors/query_user_m2m_exists_v13.json similarity index 94% rename from tests/fixtures/ir_vectors/query_user_m2m_exists_v12.json rename to tests/fixtures/ir_vectors/query_user_m2m_exists_v13.json index 330e8c4..bd7aa4b 100644 --- a/tests/fixtures/ir_vectors/query_user_m2m_exists_v12.json +++ b/tests/fixtures/ir_vectors/query_user_m2m_exists_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_m2m_exists_v12", + "vector_name": "query_user_m2m_exists_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_merge_set_v12.json b/tests/fixtures/ir_vectors/query_user_merge_set_v13.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_merge_set_v12.json rename to tests/fixtures/ir_vectors/query_user_merge_set_v13.json index b785036..02bcbe8 100644 --- a/tests/fixtures/ir_vectors/query_user_merge_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_merge_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_merge_set_v12", + "vector_name": "query_user_merge_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_mixed_set_v12.json b/tests/fixtures/ir_vectors/query_user_mixed_set_v13.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_mixed_set_v12.json rename to tests/fixtures/ir_vectors/query_user_mixed_set_v13.json index fa22d02..f81bf1f 100644 --- a/tests/fixtures/ir_vectors/query_user_mixed_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_mixed_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_mixed_set_v12", + "vector_name": "query_user_mixed_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_not_compound_v12.json b/tests/fixtures/ir_vectors/query_user_not_compound_v13.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_not_compound_v12.json rename to tests/fixtures/ir_vectors/query_user_not_compound_v13.json index 934a34f..12d1903 100644 --- a/tests/fixtures/ir_vectors/query_user_not_compound_v12.json +++ b/tests/fixtures/ir_vectors/query_user_not_compound_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_not_compound_v12", + "vector_name": "query_user_not_compound_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_not_leaf_v12.json b/tests/fixtures/ir_vectors/query_user_not_leaf_v13.json similarity index 92% rename from tests/fixtures/ir_vectors/query_user_not_leaf_v12.json rename to tests/fixtures/ir_vectors/query_user_not_leaf_v13.json index 76b95d7..8d59e19 100644 --- a/tests/fixtures/ir_vectors/query_user_not_leaf_v12.json +++ b/tests/fixtures/ir_vectors/query_user_not_leaf_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_not_leaf_v12", + "vector_name": "query_user_not_leaf_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_now_set_v12.json b/tests/fixtures/ir_vectors/query_user_now_set_v13.json similarity index 90% rename from tests/fixtures/ir_vectors/query_user_now_set_v12.json rename to tests/fixtures/ir_vectors/query_user_now_set_v13.json index eecd592..9ec37e7 100644 --- a/tests/fixtures/ir_vectors/query_user_now_set_v12.json +++ b/tests/fixtures/ir_vectors/query_user_now_set_v13.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_now_set_v12", + "vector_name": "query_user_now_set_v13", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 12, + "ir_version": 13, "payload": { "model_name": "User", "where": [ diff --git a/tests/test_after_position.py b/tests/test_after_position.py new file mode 100644 index 0000000..0de9f36 --- /dev/null +++ b/tests/test_after_position.py @@ -0,0 +1,188 @@ +"""``after(position)`` on non-null root order keys (#393, ADR-0018). + +Build-time tests need no database. E2e tests run on the backend matrix. +""" + +from datetime import UTC, datetime +from typing import Annotated + +import pytest + +import ferro +from ferro import FerroField, Model +from ferro.query.wire import compile_query + + +class AfterPageItem(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + updated_at: datetime + name: str + + +def _ordered(item: AfterPageItem): + return ( + AfterPageItem.select() + .order_by(lambda row: row.updated_at) + .order_by(lambda row: row.id) + ) + + +# --------------------------------------------------------------------------- +# Build-time (no DB). +# --------------------------------------------------------------------------- + + +def test_after_without_pk_in_order_keys_raises(): + with pytest.raises(ValueError, match=r"primary key"): + AfterPageItem.select().order_by(lambda row: row.updated_at).after( + (datetime(2026, 1, 1, tzinfo=UTC),) + ) + + +def test_after_on_pkless_model_raises(): + class AfterPkLess(Model): + name: str + rank: int = 0 + + with pytest.raises(ValueError, match=r"AfterPkLess.*no primary-key"): + AfterPkLess.select().order_by(lambda row: row.rank).order_by( + lambda row: row.name + ).after((1, "x")) + + +def test_position_of_on_pkless_model_raises(): + class AfterPkLessPos(Model): + name: str + + row = AfterPkLessPos(name="x") + with pytest.raises(ValueError, match=r"AfterPkLessPos.*no primary-key"): + AfterPkLessPos.select().order_by(lambda r: r.name).position_of(row) + + +def test_after_nullable_order_key_raises(): + class AfterNullableKey(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str | None = None + + with pytest.raises(ValueError, match=r"nullable"): + AfterNullableKey.select().order_by(lambda row: row.label).order_by( + lambda row: row.id + ).after(("x", 1)) + + +def test_after_wrong_arity_raises(): + with pytest.raises(ValueError, match=r"arity|values"): + _ordered(AfterPageItem).after((datetime(2026, 1, 1, tzinfo=UTC),)) + + +def test_after_none_in_a_slot_raises(): + with pytest.raises(ValueError, match=r"None"): + _ordered(AfterPageItem).after((datetime(2026, 1, 1, tzinfo=UTC), None)) + + +def test_after_plus_offset_raises(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + with pytest.raises(ValueError, match=r"offset"): + _ordered(AfterPageItem).offset(1).after((ts, 1)) + with pytest.raises(ValueError, match=r"offset"): + _ordered(AfterPageItem).after((ts, 1)).offset(1) + + +def test_after_is_immutable(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + base = _ordered(AfterPageItem) + paged = base.after((ts, 1)).limit(2) + assert paged is not base + assert base._after is None + assert paged._after == (ts, 1) + assert paged._limit == 2 + assert base._limit is None + + +def test_update_and_delete_reject_after(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + query = _ordered(AfterPageItem).after((ts, 1)) + with pytest.raises(ValueError, match=r"after"): + compile_query(query, "update", assignments={"name": "x"}) + with pytest.raises(ValueError, match=r"after"): + compile_query(query, "delete") + + +def test_count_drops_after_from_the_wire(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + query = _ordered(AfterPageItem).after((ts, 1)).limit(3) + payload = compile_query(query, "count").payload.to_ir_dict() + assert "after" not in payload + assert payload["limit"] is None + assert payload["offset"] is None + + +# --------------------------------------------------------------------------- +# E2e: exclusive next page in declared order. +# --------------------------------------------------------------------------- + + +async def _seed_page_items() -> list[AfterPageItem]: + rows = [ + AfterPageItem(id=1, updated_at=datetime(2026, 1, 1, tzinfo=UTC), name="a"), + AfterPageItem(id=2, updated_at=datetime(2026, 1, 1, tzinfo=UTC), name="b"), + AfterPageItem(id=3, updated_at=datetime(2026, 2, 1, tzinfo=UTC), name="c"), + AfterPageItem(id=4, updated_at=datetime(2026, 3, 1, tzinfo=UTC), name="d"), + AfterPageItem(id=5, updated_at=datetime(2026, 3, 1, tzinfo=UTC), name="e"), + ] + for row in rows: + await row.save() + return rows + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_after_returns_next_n_exclusive_in_declared_order(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_page_items() + ordered = await _ordered(AfterPageItem).all() + assert [row.id for row in ordered] == [1, 2, 3, 4, 5] + + page = ( + await _ordered(AfterPageItem) + .after((ordered[1].updated_at, ordered[1].id)) + .limit(2) + .all() + ) + assert [row.id for row in page] == [3, 4] + assert ordered[1] not in page + + past_end = ( + await _ordered(AfterPageItem) + .after((ordered[-1].updated_at, ordered[-1].id)) + .limit(2) + .all() + ) + assert past_end == [] + + # count() drops paging the same way it drops limit/offset. + assert ( + await _ordered(AfterPageItem) + .after((ordered[1].updated_at, ordered[1].id)) + .count() + == 5 + ) + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_position_of_matches_tuple_and_after_row_equals_after_tuple(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_page_items() + query = _ordered(AfterPageItem) + rows = await query.all() + anchor = rows[1] + position = query.position_of(anchor) + assert position == (anchor.updated_at, anchor.id) + + from_tuple = await query.after(position).limit(2).all() + from_row = await query.after(anchor).limit(2).all() + assert [row.id for row in from_tuple] == [3, 4] + assert [row.id for row in from_row] == [3, 4] diff --git a/tests/test_ir_vectors_contract.py b/tests/test_ir_vectors_contract.py index 23b3b4b..a1f6716 100644 --- a/tests/test_ir_vectors_contract.py +++ b/tests/test_ir_vectors_contract.py @@ -11,9 +11,9 @@ VECTORS_DIR = Path(__file__).parent / "fixtures" / "ir_vectors" SUPPORTED_DOMAINS = {"schema", "query", "codec"} -# `query` is on ir_version 12 (#392 — explicit nulls on every order_by term); +# `query` is on ir_version 13 (#393 — optional `after` position bound); # `schema`/`codec` remain v1. -SUPPORTED_IR_VERSIONS = {"schema": 1, "query": 12, "codec": 1} +SUPPORTED_IR_VERSIONS = {"schema": 1, "query": 13, "codec": 1} QUERY_OPERATORS = {"==", "!=", "<", "<=", ">", ">=", "IN", "LIKE", "AND", "OR"} MATERIALIZATION_KINDS = {"root_instances", "record", "instances"} AGGREGATE_FNS = {"count", "sum", "avg", "min", "max"} @@ -302,6 +302,19 @@ def _validate_query_payload(payload: dict[str, Any], label: str) -> None: assert isinstance(payload["offset"], int) and payload["offset"] >= 0, ( f"{label}.offset must be null or non-negative int" ) + if "after" in payload: + after = payload["after"] + assert isinstance(after, list) and after, f"{label}.after must be a non-empty list" + assert len(after) == len(payload["order_by"]), ( + f"{label}.after arity must match order_by" + ) + for i, value in enumerate(after): + value_label = f"{label}.after[{i}]" + assert isinstance(value, dict), f"{value_label} must be object" + _require_keys(value, {"kind", "value"}, value_label) + assert not payload.get("offset"), ( + f"{label} cannot carry after and a non-null offset" + ) if payload["m2m"] is not None: assert isinstance(payload["m2m"], dict), f"{label}.m2m must be null or object" joins = payload["joins"] diff --git a/tests/test_mutation_pagination_guard.py b/tests/test_mutation_pagination_guard.py index 8c156d7..dd45fdc 100644 --- a/tests/test_mutation_pagination_guard.py +++ b/tests/test_mutation_pagination_guard.py @@ -59,6 +59,7 @@ def test_mutating_payload_omits_pagination_keys(): payload = compile_query(query, operation).payload.to_ir_dict() assert "limit" not in payload assert "offset" not in payload + assert "after" not in payload assert payload["model_name"] == PaginationGuardItem.__ferro_identity__ assert payload["order_by"] == [] assert payload["m2m"] is None diff --git a/tests/test_order_by_nulls_wire.py b/tests/test_order_by_nulls_wire.py index 92a7c6a..f67ee6e 100644 --- a/tests/test_order_by_nulls_wire.py +++ b/tests/test_order_by_nulls_wire.py @@ -26,7 +26,7 @@ class WireCard(Model): .order_by(lambda c: c.updated_at, "desc") ) envelope = json.loads(compile_query(query, "fetch").wire_json) - assert envelope["ir_version"] == 12 + assert envelope["ir_version"] == 13 order_by = envelope["payload"]["order_by"] assert order_by == [ { @@ -54,7 +54,7 @@ class WireItem(Model): lambda t: {"note": t.note, "total": t.amount.sum()} ).order_by("total", "desc", nulls="first") envelope = json.loads(compile_query(query, "fetch").wire_json) - assert envelope["ir_version"] == 12 + assert envelope["ir_version"] == 13 assert envelope["payload"]["order_by"] == [ { "column": "total", @@ -99,7 +99,7 @@ class WireNative(Model): "fetch", ).wire_json ) - assert envelope["ir_version"] == 12 + assert envelope["ir_version"] == 13 entry = envelope["payload"]["order_by"][0] assert entry == { "column": "age", @@ -109,7 +109,7 @@ class WireNative(Model): } -def test_compile_query_omitted_nulls_emits_last_at_ir_version_12(): +def test_compile_query_omitted_nulls_emits_last_at_ir_version_13(): class WirePlain(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None age: int = 0 @@ -117,7 +117,7 @@ class WirePlain(Model): envelope = json.loads( compile_query(WirePlain.select().order_by("age"), "fetch").wire_json ) - assert envelope["ir_version"] == 12 + assert envelope["ir_version"] == 13 entry = envelope["payload"]["order_by"][0] assert entry == { "column": "age", diff --git a/tests/test_query_builder.py b/tests/test_query_builder.py index 8c58269..5076f7c 100644 --- a/tests/test_query_builder.py +++ b/tests/test_query_builder.py @@ -12,6 +12,7 @@ from ferro.query.nodes import FieldProxy, _serialize_query_value from ferro.query.wire import compile_query from pydantic import Field +from pydantic_core import to_json pytestmark = pytest.mark.backend_matrix @@ -40,7 +41,7 @@ def test_serialize_query_value_normalizes_non_json_native_values(): assert serialized["id"] == str(uid) assert serialized["price"] == "12.50" - assert serialized["happened_at"] == happened_at.isoformat() + assert serialized["happened_at"] == json.loads(to_json(happened_at)) assert serialized["day"] == "2026-04-24" assert serialized["status"] == QueryStatus.ACTIVE assert serialized["nested"]["ids"] == [str(uid)] @@ -67,7 +68,7 @@ class WireM2mPost(Model): assert query._m2m_context.source_id == source_id assert isinstance(query._m2m_context.source_id, uuid.UUID) assert payload["ir_kind"] == "query" - assert payload["ir_version"] == 12 + assert payload["ir_version"] == 13 assert payload["payload"]["m2m"]["source_id"] == str(source_id) @@ -214,14 +215,18 @@ class FilterUser(Model): assert {r.username for r in results} == {"taylor", "alice"} # 2. Test IN filter - results_in = await FilterUser.where(lambda t: t.username << ["jeff", "alice"]).all() + results_in = await FilterUser.where( + lambda t: t.username << ["jeff", "alice"] + ).all() assert len(results_in) == 2 assert {r.username for r in results_in} == {"jeff", "alice"} # 3. Test combined filters (Chaining) - results_chained = await FilterUser.where(lambda t: t.age < 35).where( - lambda t: t.age > 20 - ).all() + results_chained = ( + await FilterUser.where(lambda t: t.age < 35) + .where(lambda t: t.age > 20) + .all() + ) assert len(results_chained) == 2 assert {r.username for r in results_chained} == {"taylor", "jeff"} @@ -335,7 +340,9 @@ class LogicUser(Model): await LogicUser(id=3, username="alice", age=35).save() # (A OR B) AND (C) - query = LogicUser.where(lambda t: (t.username == "jeff") | (t.username == "alice")) + query = LogicUser.where( + lambda t: (t.username == "jeff") | (t.username == "alice") + ) query = query.where(lambda t: t.age > 30) results = await query.all() diff --git a/tests/test_query_immutability.py b/tests/test_query_immutability.py index b460b36..22aeaf8 100644 --- a/tests/test_query_immutability.py +++ b/tests/test_query_immutability.py @@ -54,6 +54,17 @@ class ImmUser3(Model): ] assert q3.order_by_clause == [] + def test_after_does_not_mutate(self): + class ImmUserAfter(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + age: int = 0 + + q1 = Query(ImmUserAfter).order_by("age").order_by("id") + q2 = q1.after((18, 1)).limit(2) + assert q1._after is None + assert q2._after == (18, 1) + assert q2 is not q1 + def test_m2m_context_is_immutable_so_clones_share_it_safely(self): class ImmUser4(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None diff --git a/tests/test_query_wire_vectors.py b/tests/test_query_wire_vectors.py index 9474487..4766030 100644 --- a/tests/test_query_wire_vectors.py +++ b/tests/test_query_wire_vectors.py @@ -290,23 +290,36 @@ def _q_card_nulls(m: dict[str, type]) -> Any: ) +def _q_user_after(m: dict[str, type]) -> Any: + return ( + m["User"] + .select() + .where(lambda u: u.active == True) # noqa: E712 + .order_by(lambda u: u.score) + .order_by(lambda u: u.id) + .after((10, 3)) + .limit(5) + ) + + CASES: list[tuple[str, Callable[[dict[str, type]], Any], str]] = [ - ("query_user_compound_v12", _q_user_compound, "User"), - ("query_user_not_leaf_v12", _q_not_leaf, "User"), - ("query_user_not_compound_v12", _q_not_compound, "User"), - ("query_account_exists_v12", _q_exists_bare, "Account"), - ("query_owner_not_exists_v12", _q_not_exists, "Owner"), - ("query_account_scoped_exists_v12", _q_scoped_exists, "Account"), - ("query_owner_nested_exists_v12", _q_nested_exists, "Owner"), - ("query_user_m2m_exists_v12", _q_m2m_exists, "User"), - ("query_transaction_traversal_v12", _q_traversal, "Transaction"), - ("query_transaction_left_join_v12", _q_left_join, "Transaction"), - ("query_transaction_include_v12", _q_include, "Transaction"), - ("query_transaction_record_v12", _q_record, "Transaction"), - ("query_transaction_traversed_record_v12", _q_traversed_record, "Transaction"), - ("query_transaction_aggregate_v12", _q_aggregate, "Transaction"), - ("query_transaction_global_aggregate_v12", _q_global_aggregate, "Transaction"), - ("query_card_nulls_v12", _q_card_nulls, "Card"), + ("query_user_compound_v13", _q_user_compound, "User"), + ("query_user_not_leaf_v13", _q_not_leaf, "User"), + ("query_user_not_compound_v13", _q_not_compound, "User"), + ("query_account_exists_v13", _q_exists_bare, "Account"), + ("query_owner_not_exists_v13", _q_not_exists, "Owner"), + ("query_account_scoped_exists_v13", _q_scoped_exists, "Account"), + ("query_owner_nested_exists_v13", _q_nested_exists, "Owner"), + ("query_user_m2m_exists_v13", _q_m2m_exists, "User"), + ("query_transaction_traversal_v13", _q_traversal, "Transaction"), + ("query_transaction_left_join_v13", _q_left_join, "Transaction"), + ("query_transaction_include_v13", _q_include, "Transaction"), + ("query_transaction_record_v13", _q_record, "Transaction"), + ("query_transaction_traversed_record_v13", _q_traversed_record, "Transaction"), + ("query_transaction_aggregate_v13", _q_aggregate, "Transaction"), + ("query_transaction_global_aggregate_v13", _q_global_aggregate, "Transaction"), + ("query_card_nulls_v13", _q_card_nulls, "Card"), + ("query_user_after_v13", _q_user_after, "User"), ] @@ -352,10 +365,19 @@ def test_count_zeroes_ordering_and_paging_but_keeps_joins( assert payload["order_by"] == [] assert payload["limit"] is None assert payload["offset"] is None + assert "after" not in payload assert len(payload["joins"]) == 1 assert payload["materialization"] == {"kind": "root_instances"} +def test_count_drops_after_bound(models: dict[str, type]) -> None: + query = models["User"].select().order_by("id").after((1,)).limit(3) + payload = _payload(query, "count") + assert "after" not in payload + assert payload["limit"] is None + assert payload["offset"] is None + + def test_count_on_a_projection_stays_root_instances( models: dict[str, type], ) -> None: @@ -378,6 +400,7 @@ def test_mutate_payload_omits_pagination_keys(models: dict[str, type]) -> None: payload = _payload(query, verb) assert "limit" not in payload assert "offset" not in payload + assert "after" not in payload assert payload["order_by"] == [] assert payload["m2m"] is None assert payload["joins"] == [] @@ -387,7 +410,7 @@ def test_mutate_payload_omits_pagination_keys(models: dict[str, type]) -> None: def test_literal_set_emission_matches_hand_authored_vector( models: dict[str, type], ) -> None: - vector = _vector("query_user_literal_set_v12") + vector = _vector("query_user_literal_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -408,7 +431,7 @@ def test_literal_set_emission_matches_hand_authored_vector( def test_mixed_set_emission_matches_hand_authored_vector( models: dict[str, type], ) -> None: - vector = _vector("query_user_mixed_set_v12") + vector = _vector("query_user_mixed_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -486,13 +509,13 @@ def test_literal_set_emits_every_json_value_kind( def test_envelope_is_versioned(models: dict[str, type]) -> None: envelope = json.loads(compile_query(models["User"].select(), "fetch").wire_json) assert envelope["ir_kind"] == "query" - assert envelope["ir_version"] == 12 + assert envelope["ir_version"] == 13 def test_binary_add_column_literal_matches_hand_authored_vector( models: dict[str, type], ) -> None: - vector = _vector("query_user_add_literal_set_v12") + vector = _vector("query_user_add_literal_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -512,7 +535,7 @@ def test_binary_add_column_literal_matches_hand_authored_vector( def test_binary_add_column_column_matches_hand_authored_vector( models: dict[str, type], ) -> None: - vector = _vector("query_user_add_columns_set_v12") + vector = _vector("query_user_add_columns_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -531,7 +554,7 @@ def test_binary_add_column_column_matches_hand_authored_vector( def test_now_set_matches_hand_authored_vector(models: dict[str, type]) -> None: - vector = _vector("query_user_now_set_v12") + vector = _vector("query_user_now_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -549,7 +572,7 @@ def test_now_set_matches_hand_authored_vector(models: dict[str, type]) -> None: def test_merge_set_matches_hand_authored_vector(models: dict[str, type]) -> None: - vector = _vector("query_user_merge_set_v12") + vector = _vector("query_user_merge_set_v13") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" From 200f5155409196fdf0dca35e6844580e41c24ac2 Mon Sep 17 00:00:00 2001 From: Taylor Date: Sun, 30 Aug 2026 11:18:39 -0400 Subject: [PATCH 2/4] fix(query): add required nulls on rust QueryIR test fixtures #392 made order_by.nulls required; three inline payloads still omitted it and cargo test failed deserialize on CI. Co-authored-by: Cursor --- src/query.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/query.rs b/src/query.rs index 198b619..855bb54 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1130,7 +1130,7 @@ mod tests { "right": {"node_kind": "leaf", "column": "name", "operator": "LIKE", "value": {"kind": "string", "value": "a%"}, "path": []}} ], - "order_by": [{"column": "age", "direction": "desc", "path": []}], + "order_by": [{"column": "age", "direction": "desc", "path": [], "nulls": "last"}], "limit": 10, "offset": 5, "m2m": null, "materialization": {"kind": "root_instances"}, "joins": [] })) .expect("payload deserializes"); @@ -1653,7 +1653,7 @@ mod tests { "right": {"node_kind": "leaf", "column": "amount", "operator": ">=", "value": {"kind": "int", "value": 100}, "path": []}} ], - "order_by": [{"column": "id", "direction": "asc", "path": []}], + "order_by": [{"column": "id", "direction": "asc", "path": [], "nulls": "last"}], "limit": 50, "offset": 0, "m2m": null, "materialization": {"kind": "root_instances"}, "joins": [ {"join_type": "inner", "path": [ @@ -2089,7 +2089,7 @@ mod tests { "to_table": "account", "to_column": "id"} ]} ], - "order_by": [{"column": "name", "direction": "asc", "path": ["account"]}] + "order_by": [{"column": "name", "direction": "asc", "path": ["account"], "nulls": "last"}] })) .expect("payload deserializes"); From d65e74f04eb32982084d4135a4a40b58a06d7a8e Mon Sep 17 00:00:00 2001 From: Taylor Date: Sun, 30 Aug 2026 11:21:37 -0400 Subject: [PATCH 3/4] fix(query): require normalized nulls on projected order_by append OrderByEntry.nulls is str after #392; the helper already normalizes before the call, so the parameter should not accept None. Co-authored-by: Cursor --- src/ferro/query/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ferro/query/builder.py b/src/ferro/query/builder.py index 579b607..7ef7f43 100644 --- a/src/ferro/query/builder.py +++ b/src/ferro/query/builder.py @@ -1476,7 +1476,7 @@ def _append_order_by( direction: str, path: tuple[str, ...], *, - nulls: str | None = None, + nulls: str, ) -> Self: """Clone with one resolved ORDER BY entry appended (#295).""" new = self._clone() From dcedaff41e6554a0d28d225af3cb54366a779131 Mon Sep 17 00:00:00 2001 From: Taylor Date: Sun, 30 Aug 2026 11:26:32 -0400 Subject: [PATCH 4/4] fix(query): expect omitted nulls last on sqlite left_join order ADR-0017 made omitted nulls= last on every dialect. The left_join order test still asserted SQLite's native NULLs-first ASC. Co-authored-by: Cursor --- tests/test_query_joins.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/tests/test_query_joins.py b/tests/test_query_joins.py index 85c4bc1..268f1fc 100644 --- a/tests/test_query_joins.py +++ b/tests/test_query_joins.py @@ -881,9 +881,11 @@ async def test_left_join_retains_relation_less_rows(db_url): @pytest.mark.asyncio async def test_left_join_null_retention_in_ordered_results(db_url): - """left_join + order_by on a RELATED column retains the NULL-FK row. NULL - placement diverges by dialect (ADR-0006: Postgres NULLs last on ASC, SQLite - first), so assert the full row set + the non-NULL order per-backend.""" + """left_join + order_by on a RELATED column retains the NULL-FK row. + + Omitted ``nulls=`` means last on both dialects (ADR-0017), so the orphan + lands after the two labeled rows. + """ await ferro.connect(db_url, auto_migrate=True) async with ferro.engines.session(): core = await _seed_core() @@ -899,15 +901,8 @@ async def test_left_join_null_retention_in_ordered_results(db_url): .all() ) ids = [r.id for r in rows] - # Full set retained (orphan kept by LEFT join). - assert set(ids) == {1, 2, 3} - # Non-NULL rows keep their relative order (label a1 < a2 → id 1 before 2). - assert ids.index(1) < ids.index(2) - # NULL-FK row's position is dialect-specific but deterministic. - if db_url.startswith("postgres"): - assert ids == [1, 2, 3] # NULLs last on ASC - else: - assert ids == [3, 1, 2] # SQLite sorts NULLs first + # Full set retained (orphan kept by LEFT join); omitted nulls= is last. + assert ids == [1, 2, 3] @pytest.mark.asyncio