diff --git a/CONTEXT.md b/CONTEXT.md index 3f5069e..ed33a92 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -88,6 +88,22 @@ _Avoid_: Select list, projection spec, hydration mode flag The single typed wire artifact a query ships to the Rust runtime — model identity, predicates, ordering, paging, joins, and exactly one materialization plan, inside a versioned envelope. Compiled only by `compile_query`; no other code assembles query wire shape. _Avoid_: Query dict, query def, payload dict +**Paging**: +The QueryIR window over matching rows: a size (`limit`) and a start. A start is either an offset or one position bound (`after` or `before`, never both). Both bounds are exclusive of the position. A limited `before` is the adjacent previous page; an unbounded `before` is every earlier row in declared order. Paging is not a predicate — it does not change which rows match — and `count()` drops it. +_Avoid_: pagination, cursor, page filter + +**Position**: +The ordered tuple of a query's order-key values that marks one row's place in that order. Two rows never share a position: the order keys include the model's primary key. A non-PK slot may be empty (`None`); the PK slot may not. `after`/`before` start the page from a position; `position_of` reads one off a model instance, or off a projected record that carries every order key. Traversed order keys require those relations populated. Not a cursor — encoding is the caller's. +_Avoid_: cursor, bookmark, page token, keyset + +**Order key**: +One term in a query's `order_by`: the column (root or traversed), its direction, and its null placement. A position holds one value per order key, in declaration order. +_Avoid_: sort field, sort column, order term + +**Null placement**: +Where NULL sort keys land for one order key: `last`, `first`, or `native` (that backend's own default — Postgres and SQLite are opposites). Omitted means `last`, so the same order on every backend. `native` is never implied. +_Avoid_: dialect default, omitted nulls + **Compiled query**: The single artifact `compile_query` returns: the QueryIR payload, its wire JSON, and the plan-scoped hop-class map, all views of one compile. The map is collected from the hop facts the payload itself carries, so wire and hop classes can never disagree; it is `None` unless the materialization plan decodes or hydrates through a hop model's class (mirroring the Rust `needs_hop_classes` guard — a both-sides double-check). No other code assembles hop classes for the FFI. _Avoid_: payload + kwargs, hop-class side-channel, wire tuple diff --git a/crates/ferro-schema-ir/src/lib.rs b/crates/ferro-schema-ir/src/lib.rs index ac28ede..d98edf9 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: 11` (unconditional, no earlier version emitted anywhere — -/// #269, #278, #285, #292, #310, #314, #376, #377, #378, #379). `set` is required +/// `ir_version: 14` (unconditional, no earlier version emitted anywhere — +/// #269, #278, #285, #292, #310, #314, #376, #377, #378, #379, #392, #393, #395). `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); @@ -237,7 +237,10 @@ pub enum CheckOperand { /// recursive `exists` node kind (existence tests, #314, ADR-0007); v8 adds /// 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``. +/// `+` / `-` and the `now` clock; v11 adds Postgres ``merge``; v12 requires +/// explicit ``nulls`` on every ``order_by`` term (#392); v13 adds an optional +/// ``after`` position bound on fetch payloads (#393); v14 adds an optional +/// ``before`` position bound (#395). #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct QueryIrPayload { /// Model class name the query targets. @@ -265,6 +268,15 @@ 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>, + /// Exclusive previous-page bound (`before`). Omitted when unset, same + /// policy as `after` (v14, #395). Combined with `after` is a loud error. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub before: Option>, /// Many-to-many join metadata JSON, deserialized into [`M2mContext`] downstream. pub m2m: Option, /// Relation joins collected from traversal (`[]` until #270 renders them). @@ -483,11 +495,10 @@ pub struct QueryOrderBy { /// Relation field names from the root model to the ordered column's table; /// `[]` = root model (#269 requires this empty until #270 renders joins). pub path: Vec, - /// Optional NULLS placement (`"first"` / `"last"`, case-insensitive in the - /// planner). Omitted on the wire when unset so existing fixtures keep - /// byte-identical round-trips (#361). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub nulls: Option, + /// Required NULLS placement (`"first"` / `"last"` / `"native"`, case- + /// insensitive in the planner). Every order_by term on the wire carries + /// this key; Python omitted ``nulls=`` compiles to ``"last"`` (#392). + pub nulls: String, } /// Predicate tree node in query IR: leaf comparison, compound AND/OR, a @@ -692,7 +703,7 @@ mod tests { #[test] fn query_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_compound_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_compound_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query fixture must parse"); let ir = parsed @@ -713,7 +724,7 @@ mod tests { #[test] fn query_literal_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_literal_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_literal_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query literal-set fixture must parse"); let ir = parsed @@ -722,7 +733,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, 11); + assert_eq!(envelope.ir_version, 14); 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 +750,7 @@ mod tests { #[test] fn query_mixed_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_mixed_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_mixed_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query mixed-set fixture must parse"); let ir = parsed @@ -748,7 +759,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, 11); + assert_eq!(envelope.ir_version, 14); 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 +781,7 @@ mod tests { #[test] fn query_add_literal_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_add_literal_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_add_literal_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query add-literal-set fixture must parse"); let ir = parsed @@ -779,7 +790,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, 11); + assert_eq!(envelope.ir_version, 14); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Add { left, right } => { match left.as_ref() { @@ -807,7 +818,7 @@ mod tests { #[test] fn query_add_columns_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_add_columns_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_add_columns_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query add-columns-set fixture must parse"); let ir = parsed @@ -816,7 +827,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, 11); + assert_eq!(envelope.ir_version, 14); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Add { left, right } => { match left.as_ref() { @@ -841,7 +852,7 @@ mod tests { #[test] fn query_now_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_now_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_now_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query now-set fixture must parse"); let ir = parsed @@ -850,7 +861,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, 11); + assert_eq!(envelope.ir_version, 14); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Now => {} other => panic!("expected now assignment, got {other:?}"), @@ -862,7 +873,7 @@ mod tests { #[test] fn query_merge_set_fixture_roundtrip() { let fixture = - include_str!("../../../tests/fixtures/ir_vectors/query_user_merge_set_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_merge_set_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query merge-set fixture must parse"); let ir = parsed @@ -871,7 +882,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, 11); + assert_eq!(envelope.ir_version, 14); match &envelope.payload.set_assignments[0].value { QueryValueExpr::Merge { left, right } => { match left.as_ref() { @@ -899,7 +910,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_not_leaf_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-leaf fixture must parse"); let ir = parsed @@ -926,7 +937,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_not_compound_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-compound fixture must parse"); let ir = parsed @@ -954,7 +965,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_account_exists_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query exists fixture must parse"); let ir = parsed @@ -986,7 +997,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_owner_not_exists_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query not-exists fixture must parse"); let ir = parsed @@ -1013,7 +1024,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_account_scoped_exists_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query scoped-exists fixture must parse"); let ir = parsed @@ -1047,7 +1058,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_owner_nested_exists_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query nested-exists fixture must parse"); let ir = parsed @@ -1078,7 +1089,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_user_m2m_exists_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query m2m-exists fixture must parse"); let ir = parsed @@ -1109,7 +1120,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_traversal_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query traversal fixture must parse"); let ir = parsed @@ -1137,7 +1148,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_left_join_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query left_join fixture must parse"); let ir = parsed @@ -1163,7 +1174,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_record_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query record fixture must parse"); let ir = parsed @@ -1362,7 +1373,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_v11.json" + "../../../tests/fixtures/ir_vectors/query_transaction_global_aggregate_v14.json" ); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("global aggregate fixture must parse"); @@ -1399,7 +1410,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_v11.json" + "../../../tests/fixtures/ir_vectors/query_transaction_traversed_record_v14.json" ); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query traversed record fixture must parse"); @@ -1440,7 +1451,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_aggregate_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query aggregate fixture must parse"); let ir = parsed @@ -1507,7 +1518,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_v11.json"); + include_str!("../../../tests/fixtures/ir_vectors/query_transaction_include_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query include fixture must parse"); let ir = parsed @@ -1574,9 +1585,8 @@ mod tests { } #[test] - fn query_order_by_nulls_optional_wire() { - // #361: optional `nulls` on an order_by term — present key round-trips; - // absent key deserializes as None and is omitted on serialize. + fn query_order_by_nulls_required_wire() { + // #392: every order_by term carries required `nulls`; absent key fails. let with: QueryOrderBy = serde_json::from_value(serde_json::json!({ "column": "pinned_at", "direction": "desc", @@ -1584,29 +1594,35 @@ mod tests { "nulls": "last" })) .expect("order_by with nulls must deserialize"); - assert_eq!(with.nulls.as_deref(), Some("last")); + assert_eq!(with.nulls, "last"); let encoded = serde_json::to_value(&with).expect("serialize with nulls"); assert_eq!(encoded["nulls"], "last"); - let without: QueryOrderBy = serde_json::from_value(serde_json::json!({ + let native: QueryOrderBy = serde_json::from_value(serde_json::json!({ + "column": "pinned_at", + "direction": "desc", + "path": [], + "nulls": "native" + })) + .expect("order_by with native nulls must deserialize"); + assert_eq!(native.nulls, "native"); + + let err = serde_json::from_value::(serde_json::json!({ "column": "pinned_at", "direction": "desc", "path": [] })) - .expect("order_by without nulls must deserialize"); - assert_eq!(without.nulls, None); - let encoded = serde_json::to_value(&without).expect("serialize without nulls"); + .expect_err("order_by without nulls must fail"); assert!( - encoded.get("nulls").is_none(), - "unset nulls must be omitted from the wire: {encoded}" + err.to_string().contains("nulls"), + "missing nulls must name the field: {err}" ); } #[test] fn query_card_nulls_fixture_roundtrip() { - // #363: golden vector with nulls on the first order_by term and the - // key absent on the second — deserialize, pin shape, round-trip. - let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_card_nulls_v11.json"); + // #392: golden vector with explicit nulls on every order_by term. + let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_card_nulls_v14.json"); let parsed: serde_json::Value = serde_json::from_str(fixture).expect("query card-nulls fixture must parse"); let ir = parsed @@ -1615,17 +1631,103 @@ 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, 11); + assert_eq!(envelope.ir_version, 14); assert_eq!(envelope.payload.order_by.len(), 2); - assert_eq!(envelope.payload.order_by[0].nulls.as_deref(), Some("last")); - assert!( - envelope.payload.order_by[1].nulls.is_none(), - "second term must omit nulls" + assert_eq!(envelope.payload.order_by[0].nulls, "last"); + assert_eq!( + 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_v14.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, 14); + 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 query_after_null_slot_fixture_roundtrip() { + let fixture = + include_str!("../../../tests/fixtures/ir_vectors/query_card_after_null_slot_v14.json"); + let parsed: serde_json::Value = + serde_json::from_str(fixture).expect("query after-null-slot 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-null-slot IR must deserialize"); + assert_eq!(envelope.ir_version, 14); + let after = envelope + .payload + .after + .as_ref() + .expect("fetch payload must carry after"); + assert_eq!(after.len(), 2); + assert_eq!(after[0].kind, "null"); + assert_eq!(after[0].value, serde_json::json!(null)); + assert_eq!(after[1].kind, "int"); + assert_eq!(after[1].value, serde_json::json!(4)); + assert_eq!(envelope.payload.order_by[0].nulls, "last"); + assert_eq!(envelope.payload.order_by[1].nulls, "last"); + let encoded = + serde_json::to_value(&envelope).expect("query after-null-slot IR must serialize"); + assert_eq!( + encoded, ir, + "query after-null-slot round-trip must not drift" + ); + } + + #[test] + fn query_before_fixture_roundtrip() { + let fixture = include_str!("../../../tests/fixtures/ir_vectors/query_user_before_v14.json"); + let parsed: serde_json::Value = + serde_json::from_str(fixture).expect("query before 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 before IR must deserialize"); + assert_eq!(envelope.ir_version, 14); + let before = envelope + .payload + .before + .as_ref() + .expect("fetch payload must carry before"); + assert_eq!(before.len(), 2); + assert_eq!(before[0].kind, "int"); + assert_eq!(before[0].value, serde_json::json!(10)); + assert_eq!(before[1].kind, "int"); + assert_eq!(before[1].value, serde_json::json!(3)); + assert!(envelope.payload.after.is_none()); + let encoded = serde_json::to_value(&envelope).expect("query before IR must serialize"); + assert_eq!(encoded, ir, "query before round-trip must not drift"); + } + #[test] fn codec_fixture_roundtrip() { let fixture = diff --git a/docs/adr/0017-omitted-nulls-means-last.md b/docs/adr/0017-omitted-nulls-means-last.md new file mode 100644 index 0000000..5d8205b --- /dev/null +++ b/docs/adr/0017-omitted-nulls-means-last.md @@ -0,0 +1,26 @@ +# Omitted `nulls=` means last, not the dialect default + +`order_by(..., nulls=)` accepts `"last"` | `"first"` | `"native"`. Omitted +means **`last`** — the same NULL placement on every backend. `"native"` is the +escape hatch for a backend's own default (Postgres and SQLite are opposites). +`native` is never implied. + +#363 shipped omitted as dialect-native and deliberately did not cross-assert +omitted DESC. That split is the opposite of portable paging: the same +`after((3pm, 2))` would start in a different bucket per backend, and a +Postgres `DESC` with no `nulls=` puts NULLs *first* (unpinned conversations +leading). Defaulting to `last` is the typical "empties at the bottom" list +and matches the Pinch pinned-first shape. + +This is a breaking change for every `order_by` that omitted `nulls=`, not +only for `after`/`before`: + +- Postgres `DESC` — NULLs move first → last +- SQLite `ASC` — NULLs move first → last + +Rejected: requiring `nulls=` only on nullable keys when paging (the kwarg +is required sometimes and not others); requiring it on every key only when +`after`/`before` is set (same smell); leaving omitted as `native` (pages +are not portable). + +See `CONTEXT.md`: Null placement. Grilled with #372. diff --git a/docs/adr/0018-position-paging.md b/docs/adr/0018-position-paging.md new file mode 100644 index 0000000..4c59e30 --- /dev/null +++ b/docs/adr/0018-position-paging.md @@ -0,0 +1,32 @@ +# `after`/`before` are position paging, not a derived predicate + +`Query.after(position)` / `Query.before(position)` are a **paging start** — +a sibling of `limit`/`offset` on the QueryIR payload — not a `where()` +predicate the query writes for itself. A position is the ordered tuple of +the query's order-key values; `position_of(row)` reads it; `after(row)` is +sugar. Cursor encoding is the caller's. + +The bound is exclusive. `after` + `offset`, or `after` + `before`, is a +build-time error (`count()` already drops paging). Order keys are root or +traversed columns (not aggregates); they must include the model's primary +key so two rows never share a position. `None` is legal in every non-PK +slot — column nullability is the wrong question, because a `left_join`'d +NOT NULL related column is still NULL when the relation is missing. + +`before(position).limit(n)` is the **adjacent previous page**, yielded in +the declared order (flip comparisons and order, fetch n, reverse). +Unbounded `before()` is every earlier row in declared order — a prefix, +not a page. Limit is optional on both sides; on unbounded `before()`, +`first()` (limit 1 → adjacent) and `all()[0]` (prefix head) disagree. +That is accepted and documented. + +Same chainers on `ProjectedQuery`. `position_of(Row)` requires every order +key to be in the projection; otherwise pass a tuple. Grouped aggregates +fail the PK-in-order-keys rule. + +Rejected: injecting the keyset tree into `where` (makes predicates depend +on `order_by`); an opaque Position type (Pinch must rebuild from a decoded +cursor); silently appending the PK (hidden extra sort); requiring `nulls=` +only when paging (ADR-0017). + +See `CONTEXT.md`: Paging, Position, Order key. Grilled with #372. diff --git a/docs/examples/partial_selects.py b/docs/examples/partial_selects.py index f9b382f..cd1f049 100644 --- a/docs/examples/partial_selects.py +++ b/docs/examples/partial_selects.py @@ -29,9 +29,9 @@ class Transaction(Model): id: int | None = Field(default=None, primary_key=True) amount: int memo: str - account: Annotated[ - Account | None, ForeignKey(related_name="transactions") - ] = None + account: Annotated[Account | None, ForeignKey(related_name="transactions")] = None + + # --8<-- [end:schema] @@ -90,9 +90,7 @@ async def main() -> None: # A selected field may reach across a relation, at any depth. # Unaliased, the field takes the bare leaf column name. rows = await ( - Transaction.select(lambda t: (t.memo, t.account.label)) - .order_by("id") - .all() + Transaction.select(lambda t: (t.memo, t.account.label)).order_by("id").all() ) assert rows[0].model_dump() == {"memo": "coffee", "label": "a1"} # --8<-- [end:traversed] @@ -165,6 +163,22 @@ async def main() -> None: assert row is not None and row.memo == "coffee" # --8<-- [end:compose] + # --8<-- [start:projected-paging] + # Projected records page the same way instances do. position_of(Row) + # requires every order key in the projection; otherwise pass a tuple. + projected = ( + Transaction.select(lambda t: {"label": t.account.label, "id": t.id}) + .order_by(lambda t: t.account.label) + .order_by(lambda t: t.id) + ) + records = await projected.all() + next_records = ( + await projected.after(projected.position_of(records[1])).limit(2).all() + ) + # --8<-- [end:projected-paging] + assert [r.id for r in records] == [1, 2, 3] + assert [r.id for r in next_records] == [3] + # --8<-- [start:count] # count()/exists() are unaffected by projection: they measure the # same matching rows a full query would. diff --git a/docs/examples/predicates.py b/docs/examples/predicates.py index fe513b6..1fcfd24 100644 --- a/docs/examples/predicates.py +++ b/docs/examples/predicates.py @@ -118,6 +118,56 @@ 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"] + + # --8<-- [start:before-paging] + previous_page = ( + await User.select() + .order_by(lambda user: user.age) + .order_by(lambda user: user.id) + .before(next_page[0]) + .limit(2) + .all() + ) + earlier = ( + await User.select() + .order_by(lambda user: user.age) + .order_by(lambda user: user.id) + .before(next_page[0]) + .all() + ) + adjacent = ( + await User.select() + .order_by(lambda user: user.age) + .order_by(lambda user: user.id) + .before(next_page[0]) + .first() + ) + # --8<-- [end:before-paging] + assert [user.name for user in previous_page] == ["dave", "bob"] + assert [user.name for user in earlier] == ["dave", "bob"] + assert adjacent is not None and adjacent.name == "bob" + assert earlier[0].name == "dave" + assert adjacent.name != earlier[0].name + t0 = datetime(2026, 1, 1, tzinfo=UTC) t1 = datetime(2026, 2, 1, tzinfo=UTC) t2 = datetime(2026, 3, 1, tzinfo=UTC) @@ -146,6 +196,28 @@ async def main() -> None: "unpinned-old", ] + # --8<-- [start:after-null-paging] + last_pinned = cards[1] + unpinned_page = ( + await Card.select() + .order_by(lambda card: card.pinned_at, "desc") + .order_by(lambda card: card.updated_at, "desc") + .order_by(lambda card: card.id, "desc") + .after(last_pinned) + .all() + ) + remaining_unpinned = ( + await Card.select() + .order_by(lambda card: card.pinned_at, "desc") + .order_by(lambda card: card.updated_at, "desc") + .order_by(lambda card: card.id, "desc") + .after((None, cards[2].updated_at, cards[2].id)) + .all() + ) + # --8<-- [end:after-null-paging] + assert [card.title for card in unpinned_page] == ["unpinned-new", "unpinned-old"] + assert [card.title for card in remaining_unpinned] == ["unpinned-old"] + # --8<-- [start:terminals] everyone = await User.all() first_admin = await User.where(lambda user: user.role == "admin").first() diff --git a/docs/examples/traversal.py b/docs/examples/traversal.py index 9bc5d8e..71a1ed6 100644 --- a/docs/examples/traversal.py +++ b/docs/examples/traversal.py @@ -10,7 +10,16 @@ import asyncio from typing import Annotated -from ferro import BackRef, Field, ForeignKey, ManyToMany, Model, Relation, connect, engines +from ferro import ( + BackRef, + Field, + ForeignKey, + ManyToMany, + Model, + Relation, + connect, + engines, +) # --8<-- [start:schema] @@ -39,6 +48,8 @@ class Transaction(Model): id: int | None = Field(default=None, primary_key=True) amount: int account: Annotated[Account, ForeignKey(related_name="transactions")] + + # --8<-- [end:schema] @@ -47,6 +58,8 @@ class Note(Model): id: int | None = Field(default=None, primary_key=True) body: str account: Annotated[Account | None, ForeignKey(related_name="notes")] = None + + # --8<-- [end:note-model] @@ -62,6 +75,8 @@ class Flight(Model): id: int | None = Field(default=None, primary_key=True) origin: Annotated[Airport, ForeignKey(related_name="departures")] destination: Annotated[Airport, ForeignKey(related_name="arrivals")] + + # --8<-- [end:two-fk-model] @@ -69,8 +84,12 @@ class Flight(Model): class Employee(Model): id: int | None = Field(default=None, primary_key=True) name: str - manager: Annotated["Employee", ForeignKey(related_name="reports", nullable=True)] = None + manager: Annotated[ + "Employee", ForeignKey(related_name="reports", nullable=True) + ] = None reports: Relation[list["Employee"]] = BackRef() + + # --8<-- [end:self-fk-model] @@ -92,6 +111,8 @@ class Post(Model): id: int | None = Field(default=None, primary_key=True) title: str tags: Relation[list["Tag"]] = ManyToMany(related_name="posts") + + # --8<-- [end:m2m-model] @@ -147,7 +168,9 @@ async def main() -> None: # --8<-- [start:pinch] top = await ( - Transaction.where(lambda transaction: transaction.account.ledger_id == ledger_a.id) + Transaction.where( + lambda transaction: transaction.account.ledger_id == ledger_a.id + ) .where(lambda transaction: transaction.amount >= 20) .order_by(lambda transaction: transaction.amount, "desc") .limit(2) @@ -188,6 +211,19 @@ async def main() -> None: # --8<-- [end:order-by] assert [r.id for r in ordered] == [1, 2, 3, 4, 5, 6] + # --8<-- [start:traversed-paging] + # after/before take a decoded tuple — include() is not required to page. + next_page = await ( + Transaction.select() + .order_by(lambda transaction: transaction.account.label) + .order_by(lambda transaction: transaction.id) + .after(("a1", 2)) + .limit(2) + .all() + ) + # --8<-- [end:traversed-paging] + assert [r.id for r in next_page] == [3, 4] + # --8<-- [start:instance-eq] # `== instance` filters by the shadow FK column, with no join. on_a1 = await Transaction.where( @@ -291,9 +327,7 @@ async def _run_m2m() -> None: # --8<-- [start:m2m-query] # The association context (post.tags) and forward-FK traversal on the tag # compose in one statement. - admin_tags = await post.tags.where( - lambda tag: tag.created_by.role == "admin" - ).all() + admin_tags = await post.tags.where(lambda tag: tag.created_by.role == "admin").all() # --8<-- [end:m2m-query] assert {t.id for t in admin_tags} == {1} diff --git a/docs/pages/api/queries.md b/docs/pages/api/queries.md index 303845f..427b09a 100644 --- a/docs/pages/api/queries.md +++ b/docs/pages/api/queries.md @@ -4,7 +4,7 @@ Prefix `~` negates **any** predicate — leaf comparison or `&`/`|` compound — rendering as SQL `NOT (...)` over the condition it wraps (ADR-0008). It is the universal negation rule: there are no per-operator negative forms (`~t.role.in_([...])` is NOT IN, `~t.email.like(p)` is NOT LIKE), and double negation nests. Like SQL `NOT` and the `!=` operator, a negated comparison excludes rows where the compared column is `NULL` — see [Negation and NULL values](../guide/queries.md#negation-and-null-values). -`where()` and `order_by()` lambdas may **traverse** a forward-FK relation (`lambda t: t.account.ledger_id == 1`): each hop renders one INNER join, deduplicated by relation path (ADR-0006). `join()` forces a join on a relation path (a bare `join()` is an existence filter on a nullable relation), and `left_join()` marks the whole path LEFT to keep relation-less rows. `order_by(..., nulls="first" | "last")` pins `NULL` placement when a sort key is nullable (dialect defaults otherwise — see [Ordering, Limit & Offset](../guide/queries.md#ordering-limit--offset)). See the [Querying Across Relationships](../guide/queries.md#querying-across-relationships) guide for worked examples. +`where()` and `order_by()` lambdas may **traverse** a forward-FK relation (`lambda t: t.account.ledger_id == 1`): each hop renders one INNER join, deduplicated by relation path (ADR-0006). `join()` forces a join on a relation path (a bare `join()` is an existence filter on a nullable relation), and `left_join()` marks the whole path LEFT to keep relation-less rows. Omitted `nulls=` on `order_by` means `NULLS LAST` on every backend; pass `nulls="first"`, `nulls="last"`, or `nulls="native"` (dialect default) to override — see [Ordering, Limit & Offset](../guide/queries.md#ordering-limit--offset). See the [Querying Across Relationships](../guide/queries.md#querying-across-relationships) guide for worked examples. A reverse (`BackRef`) or many-to-many relation in a predicate supports exactly one verb — the **existence test** `t.rel.exists(inner_lambda=None)` (ADR-0007). It renders as a correlated `EXISTS` at every cardinality (never a join, so the result stays root-shaped and each matching root returns once), negates with `~`, and the optional inner lambda is a full ferro predicate over the related model (operators, `&`/`|`/`~`, forward traversal rendered inside the subquery, nested tests). Everything else on a reverse edge — column access, comparisons (including `!= None`), `in_` (including a query RHS), `join()`/`left_join()` — raises at build time naming `.exists()`; an inner lambda referencing any scope but its own parameter is likewise rejected ([#309](https://github.com/syn54x/ferro-orm/issues/309)). See [Existence Tests](../guide/queries.md#existence-tests-on-reverse-many-to-many-relations) for worked examples. diff --git a/docs/pages/guide/queries.md b/docs/pages/guide/queries.md index 4cdbd52..0146037 100644 --- a/docs/pages/guide/queries.md +++ b/docs/pages/guide/queries.md @@ -166,7 +166,7 @@ For a row where `amount` is `NULL`, `amount > 100` is unknown, `NOT unknown` is ## Ordering, Limit & Offset -Sort with `.order_by(field, direction, *, nulls=...)` (direction defaults to ascending; pass `"desc"` to reverse) and slice with `.limit()` / `.offset()`. Pass `nulls="first"` or `nulls="last"` to pin where `NULL` sort keys land. `field` is a lambda naming the column (`order_by(lambda u: u.created_at, "desc")`, matching the `where()` predicate style) or a column-name string (`order_by("created_at", "desc")`). Both forms are validated against the model's queryable columns at build time. +Sort with `.order_by(field, direction, *, nulls=...)` (direction defaults to ascending; pass `"desc"` to reverse) and slice with `.limit()` / `.offset()`. Omitted `nulls=` means `NULL`s sort last on every backend; pass `nulls="first"` to lead with `NULL`s, or `nulls="native"` for each dialect's default placement. `field` is a lambda naming the column (`order_by(lambda u: u.created_at, "desc")`, matching the `where()` predicate style) or a column-name string (`order_by("created_at", "desc")`). Both forms are validated against the model's queryable columns at build time. A pinned-first list is the usual reason to care — put unpinned cards (`pinned_at IS NULL`) after pinned ones, then break ties by recency: @@ -190,13 +190,37 @@ A pinned-first list is the usual reason to care — put unpinned cards (`pinned_ ORDER BY pinned_at DESC NULLS LAST, updated_at DESC, id DESC ``` -Omitting `nulls=` on a nullable sort key leaves placement to the dialect: PostgreSQL `DESC` puts `NULL`s first; SQLite `DESC` puts them last. Pass `nulls=` when you care. +Omitting `nulls=` on a nullable sort key means `NULLS LAST` on every backend. Pass `nulls="first"` to lead with `NULL`s, or `nulls="native"` when you want each dialect's default (PostgreSQL and SQLite disagree on `DESC`). ```python --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 and the order keys must include the primary key. `None` is legal in every non-PK slot — that is how a pinned-first list continues through unpinned rows: + +```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 the pinned-first shape (`order_by(pinned_at, "desc")`, omitted `nulls=` → last), `after((None, id))` continues through the remaining unpinned rows: + +```python +--8<-- "docs/examples/predicates.py:after-null-paging" +``` + +To page backward, `.before(position)` is the other start. With a limit it is the **adjacent previous page**, still yielded in the declared order. Without a limit it is every earlier row in declared order — a prefix, not a page. The bound is exclusive. `after` and `before` cannot share a query. + +```python +--8<-- "docs/examples/predicates.py:before-paging" +``` + +On unbounded `before()`, `first()` and `all()[0]` disagree: `first()` is `limit(1)` (the adjacent previous row) and `all()[0]` is the head of the prefix (the earliest earlier row). That is accepted. + +The same chainers work when an order key is a related column (`order_by(lambda t: t.account.label)`) or the query is a projected record: pass a tuple of the order-key values. `position_of` on a model instance requires those relations populated; `position_of` on a `Row` requires every order key to be in the projection. + +For robust pagination patterns, see [Pagination](../howto/pagination.md). ## Executing Queries @@ -308,6 +332,12 @@ The narrowing is query-wide, not per-clause: the join is rendered once for the w Because the sort join is INNER too, ordering by a related column drops relation-less rows — the same narrowing as `where()`. (Use `left_join` to keep them; see below.) +`after()` / `before()` take a decoded **tuple** of those order-key values — `include()` is not required to page. `position_of` on a model instance does require the relation populated. + +```python +--8<-- "docs/examples/traversal.py:traversed-paging" +``` + ### One join per relation path The relation path is the join's identity. Reference the same path in two `where()` calls, in an `&`/`|` tree, or across `where()` and `order_by()`, and it renders as **one** join. Distinct paths — even to the same table — render as distinct joins. There is no alias to name and none to manage; the path does that job. @@ -366,8 +396,8 @@ When you want the relation-less rows *kept* rather than filtered out, opt into a A bare `left_join` on a path also traversed by `where()` lifts the shared edge to LEFT — an explicit LEFT always beats an implicit INNER on the same edge (declaring `join` **and** `left_join` on one edge is a build-time `ValueError`). -!!! note "NULL ordering is dialect-defined unless you set `nulls=`" - Relation-less rows under `left_join` land as `NULL` sort keys on a related column — the same dialect-default split that hits a nullable **root** column. Pass `nulls="first"` or `nulls="last"` when you care — see [Ordering, Limit & Offset](#ordering-limit--offset). +!!! note "NULL ordering defaults to last unless you override it" + Relation-less rows under `left_join` land as `NULL` sort keys on a related column — omitted `nulls=` still means last. Pass `nulls="first"` to lead with `NULL`s, or `nulls="native"` for dialect-default placement — see [Ordering, Limit & Offset](#ordering-limit--offset). ### Two foreign keys to the same table @@ -782,12 +812,16 @@ Projection traversal is ordinary traversal (ADR-0006): it renders an INNER join ### Projections compose like any other query -`where()` (relation traversal included), `order_by()` (even by columns the projection does not select), `limit()`/`offset()`, and `first()` all work unchanged; on a plain projection `count()` and `exists()` are unaffected — they measure the same matching rows a full query would. (On an *aggregate* projection they raise with guidance instead — see [Aggregations & Grouped Queries](aggregations.md#the-loud-limits).) +`where()` (relation traversal included), `order_by()` (even by columns the projection does not select), `limit()`/`offset()`, `after()`/`before()`, and `first()` all work unchanged; on a plain projection `count()` and `exists()` are unaffected — they measure the same matching rows a full query would. (On an *aggregate* projection they raise with guidance instead — see [Aggregations & Grouped Queries](aggregations.md#the-loud-limits).) `position_of` on a `Row` requires every order key in the projection; otherwise pass a tuple. ```python --8<-- "docs/examples/partial_selects.py:compose" ``` +```python +--8<-- "docs/examples/partial_selects.py:projected-paging" +``` + ```python --8<-- "docs/examples/partial_selects.py:count" ``` diff --git a/docs/solutions/patterns/position-paging-after.md b/docs/solutions/patterns/position-paging-after.md new file mode 100644 index 0000000..6872430 --- /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, 396] +captured: 2026-08-30 +--- + +## Problem + +`after(position)` / `before(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 folds each key's direction, null placement, and whether the bound is NULL into the same tree so a cursor can cross the NULL bucket in one query. #395's `before()` inverts each order key (`asc`↔`desc`, `first`↔`last`, `"native"` stays `"native"`), runs that same tree, fetches, then reverses the hydrated rows so the caller sees the declared order. + +## 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. #395 does not add a second expander either — `before_condition` inverts keys then calls the same function. `"native"` resolves inside that function from `Dialect` (Postgres: NULL is larger; SQLite: NULL is smaller). + +Python validates the wedge at `after()` / `before()` / `position_of()` (root or traversed columns, PK included; `None` legal in every non-PK slot) and `compile_query` is the only assembler that puts `after` / `before` on the fetch payload as typed `kind`/`value` nodes, including `kind: "null"`. Count omits the keys; mutations reject them. Column nullability is not consulted — a `left_join`'d NOT NULL related column may still be NULL when the relation is missing. Do not invent a second expander: path-carrying `order_by` terms already go through `qualify_column_with_joins`. + +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 d3e620c..f53481f 100644 --- a/src/ferro/query/builder.py +++ b/src/ferro/query/builder.py @@ -64,13 +64,40 @@ _ROWS_OF_ROW: type[Rows[Row]] = Rows[Row] -def _normalize_order_by_nulls(nulls: str | None) -> str | None: - """Lower-case and validate ``nulls=``; ``None`` stays omitted on the wire.""" +def _dotted_order_key(entry: OrderByEntry) -> str: + """``account.label`` for a traversed key, ``id`` for a root key.""" + return ".".join((*entry.path, entry.column)) + + +def _read_instance_order_value(row: Any, entry: OrderByEntry) -> Any: + """Walk a populated relation path; unpopulated is a loud build-time error. + + Include-population puts the relation name in the instance ``__dict__``, + shadowing the class-level ``ForwardDescriptor``. A missing key is the + awaitable (unpopulated) contract — do not follow the descriptor. + """ + current = row + for hop in entry.path: + if hop not in vars(current): + dotted = _dotted_order_key(entry) + raise ValueError( + f"position_of() requires relation path {dotted!r} populated " + "on the instance; pass a tuple if you only have the values" + ) + hop_value = vars(current)[hop] + if hop_value is None: + return None + current = hop_value + return getattr(current, entry.column) + + +def _normalize_order_by_nulls(nulls: str | None) -> str: + """Lower-case and validate ``nulls=``; omitted means ``last`` on the wire (#392).""" if nulls is None: - return None + return "last" normalized = nulls.lower() - if normalized not in ("first", "last"): - raise ValueError("nulls must be 'first' or 'last'") + if normalized not in ("first", "last", "native"): + raise ValueError("nulls must be 'first', 'last', or 'native'") return normalized @@ -555,6 +582,8 @@ 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._before: 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() @@ -797,8 +826,9 @@ def order_by( field: Column selector — lambda receiving a :class:`QueryProxy`, or a column-name string. direction: ``"asc"`` (default) or ``"desc"``. - nulls: Optional ``"first"`` or ``"last"`` null placement (keyword- - only). Omitted when unset. + nulls: Optional ``"first"``, ``"last"``, or ``"native"`` null + placement (keyword-only). Omitted means ``"last"``; ``"native"`` + keeps each backend's dialect default. Returns: A new ``Query`` with the ordering added; ``self`` is unchanged. @@ -809,7 +839,7 @@ def order_by( single column reference (a bare relation, e.g. ``lambda t: t.account``, is meaningless as a sort key). ValueError: If ``direction`` is not ``"asc"`` or ``"desc"``, or - ``nulls`` is not ``"first"`` or ``"last"``. + ``nulls`` is not ``"first"``, ``"last"``, or ``"native"``. Examples: >>> newest = await Post.select().order_by(lambda p: p.created_at, "desc").all() @@ -1028,15 +1058,183 @@ def offset(self, value: int) -> Self: Returns: A new ``Query`` with the clause added; ``self`` is unchanged. + Raises: + ValueError: If ``after()`` or ``before()`` is already set — a query + has one start. + Examples: >>> query = User.select().offset(20) >>> query._offset 20 """ + if self._after is not None or self._before is not None: + raise ValueError( + "after()/before() 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_position_order_keys(self) -> None: + """Require order keys that include the root primary key (#393/#396). + + Root and traversed columns are both legal. A related column named + ``id`` is not the model's primary key — only a root PK slot counts. + Aggregate output names are rejected even when the PK is also a + group key. + """ + 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()/before()/position_of() require one." + ) + if not self.order_by_clause: + raise ValueError( + "after()/before() require order_by() keys that include the " + "model's primary key" + ) + pk_seen = False + for entry in self.order_by_clause: + if not entry.path and entry.column == pk: + pk_seen = True + if not pk_seen: + raise ValueError( + "after()/before() require the model's primary key in the " + "order keys; Ferro will not append it silently" + ) + projection = getattr(self, "_projection", None) + if projection: + agg_names = {field.name for field in projection if field.agg} + for entry in self.order_by_clause: + if not entry.path and entry.column in agg_names: + raise ValueError( + "after()/before()/position_of() do not support " + f"aggregate order key {entry.column!r}" + ) + + 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 position-paging + set, or a traversed order key's relation is unpopulated. + """ + 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_position_order_keys() + return tuple( + _read_instance_order_value(row, entry) for entry in self.order_by_clause + ) + + def _resolve_position( + self, position: tuple[Any, ...] | T, *, op: str + ) -> tuple[Any, ...]: + """Validate a position bound for ``after()`` / ``before()``.""" + if self._offset is not None: + raise ValueError( + f"{op}() cannot be combined with offset(): a query has one " + "start (an offset or a position bound, never both)." + ) + if op == "after" and self._before is not None: + raise ValueError( + "after() cannot be combined with before(): a query has one start." + ) + if op == "before" and self._after is not None: + raise ValueError( + "after() cannot be combined with before(): a query has one start." + ) + if isinstance(position, tuple): + self._assert_position_order_keys() + expected = len(self.order_by_clause) + if len(position) != expected: + raise ValueError( + f"{op}() expected {expected} position values " + f"(one per order_by key), got {len(position)}" + ) + bound = position + elif isinstance(position, (self.model_cls, Row)): + bound = self.position_of(position) + else: + raise TypeError( + f"{op}() expected a position tuple or a model instance, " + f"got {type(position).__name__}" + ) + pk = getattr(self.model_cls, "__ferro_pk__", None) + for entry, value in zip(self.order_by_clause, bound, strict=True): + if not entry.path and entry.column == pk and value is None: + raise ValueError( + f"{op}() does not support None in the primary-key position slot" + ) + return bound + + 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 + may be root or traversed columns and must include the primary key. + ``None`` is legal in every non-PK slot; the PK slot cannot be empty. + + 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, the PK slot is ``None``, + ``offset()`` is already set, or ``before()`` is already set. + """ + bound = self._resolve_position(position, op="after") + new = self._clone() + new._after = bound + return new + + def before(self, position: tuple[Any, ...] | T) -> Self: + """Start at the rows immediately before an exclusive position. + + With a limit this is the adjacent previous page, yielded in the + declared order. Without a limit it is every earlier row in declared + order (a prefix, not a page). On unbounded ``before()``, ``first()`` + is the adjacent previous row and ``all()[0]`` is the head of the + prefix — they disagree, and that is accepted. + + ``before(row)`` is sugar for ``before(position_of(row))``. Same + order-key rules as ``after()`` (root or traversed columns, PK + required). Cannot be combined with ``after()`` or ``offset()``. + + 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, the tuple has the + wrong arity, the PK slot is ``None``, ``offset()`` is + already set, or ``after()`` is already set. + """ + bound = self._resolve_position(position, op="before") + new = self._clone() + new._before = bound + return new + async def all(self) -> list[T]: """Return all model instances that match the current query @@ -1050,12 +1248,15 @@ async def all(self) -> list[T]: """ compiled = compile_query(self, "fetch") route = await self._transaction_or_using() - return await fetch_filtered( + rows = await fetch_filtered( self.model_cls, compiled.wire_json, route, hop_classes=compiled.hop_classes, ) + if self._before is not None: + rows.reverse() + return rows async def count(self) -> int: """Return the number of records that match the current query @@ -1361,7 +1562,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() @@ -1403,8 +1604,9 @@ def order_by( field: Output field name or root column-name string, or a lambda naming a source column / aggregate expression. direction: ``"asc"`` (default) or ``"desc"``. - nulls: Optional ``"first"`` or ``"last"`` null placement (keyword- - only). Omitted when unset. + nulls: Optional ``"first"``, ``"last"``, or ``"native"`` null + placement (keyword-only). Omitted means ``"last"``; ``"native"`` + keeps each backend's dialect default. Returns: A new query with the ordering added; ``self`` is unchanged. @@ -1552,6 +1754,50 @@ def exists(self): # type: ignore[override] ) return super().exists() + def position_of(self, row: T | Row) -> tuple[Any, ...]: + """Read this query's order-key tuple off a projected record or instance. + + Every order key must be in the projection (matched by source column + and path, or by output name). Otherwise pass a tuple to + ``after()``/``before()``. + + Raises: + TypeError: If ``row`` is neither a :class:`Row` nor a model + instance of the queried model. + ValueError: If an order key is missing from the projection, the + order keys are illegal, or a traversed key on a model + instance is unpopulated. + """ + if isinstance(row, Row): + self._assert_position_order_keys() + return tuple( + self._read_projected_order_value(row, entry) + for entry in self.order_by_clause + ) + return super().position_of(row) + + def _projected_name_for_order_key(self, entry: OrderByEntry) -> str | None: + """Output field name that carries this order key, or ``None``.""" + name_match: str | None = None + for field in self._projection: + if field.agg: + continue + if field.column == entry.column and field.path == entry.path: + return field.name + if not entry.path and field.name == entry.column: + name_match = field.name + return name_match + + def _read_projected_order_value(self, row: Row, entry: OrderByEntry) -> Any: + name = self._projected_name_for_order_key(entry) + if name is None: + dotted = _dotted_order_key(entry) + raise ValueError( + f"position_of() cannot read order key {dotted!r} from the " + "projection; select it, or pass a tuple" + ) + return getattr(row, name) + async def all(self) -> Rows[Row]: # type: ignore[override] # ty: ignore[invalid-method-override] """Return the projected records for every matching row. @@ -1573,6 +1819,8 @@ async def all(self) -> Rows[Row]: # type: ignore[override] # ty: ignore[invali record_cls=Row, hop_classes=compiled.hop_classes, ) + if self._before is not None: + records.reverse() return _ROWS_OF_ROW._wrap(records) async def first(self) -> Row | None: # type: ignore[override] # ty: ignore[invalid-method-override] 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 8701fb0..089ccf7 100644 --- a/src/ferro/query/wire.py +++ b/src/ferro/query/wire.py @@ -10,11 +10,13 @@ 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``, +``_before``, +``_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 +52,10 @@ # mutate arm; they stay distinct values so guardrail errors name the caller. QueryVerb = Literal["fetch", "count", "update", "delete"] -# Always 11 (#379 — unconditional bump, exactly like v10 at #378). v11 -# adds the ``merge`` SET value-expression kind. Python and Rust ship in -# one wheel, so a single supported version is the whole contract. -_IR_VERSION = 11 +# Always 14 (#395 — unconditional bump). v14 adds an optional ``before`` +# position bound on fetch payloads. Python and Rust ship in one wheel, so a +# single supported version is the whole contract. +_IR_VERSION = 14 class _AbsentType: @@ -128,17 +130,15 @@ class OrderByEntry: column: str direction: str path: tuple[str, ...] - nulls: str | None = None + nulls: str = "last" def to_ir_dict(self) -> dict[str, Any]: - payload: dict[str, Any] = { + return { "column": self.column, "direction": self.direction, "path": list(self.path), + "nulls": self.nulls, } - if self.nulls is not None: - payload["nulls"] = self.nulls - return payload @dataclass(frozen=True) @@ -343,7 +343,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`` / ``before`` are + omitted when unset (fetch, count, and mutating payloads alike). """ model_name: str @@ -352,6 +353,8 @@ class QueryIrPayload: order_by: tuple[OrderByEntry, ...] limit: int | None | _AbsentType offset: int | None | _AbsentType + after: tuple[QueryValue, ...] | None + before: tuple[QueryValue, ...] | None m2m: M2mContext | None joins: tuple[QueryJoin, ...] materialization: Materialization @@ -367,6 +370,10 @@ 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] + if self.before: + payload["before"] = [value.to_ir_dict() for value in self.before] 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() @@ -798,23 +805,63 @@ def _recipe_set( return tuple(compiled) +def _after_query_values(position: tuple[Any, ...]) -> tuple[QueryValue, ...]: + """Typed query-value nodes for one position 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_position( + query: "Query[Any]", bound: tuple[Any, ...] | None, *, op: str +) -> tuple[QueryValue, ...] | None: + """Serialize a fetch payload's ``after`` / ``before`` bound.""" + if bound is None: + return None + query._assert_position_order_keys() + if len(bound) != len(query.order_by_clause): + raise ValueError( + f"{op}() expected {len(query.order_by_clause)} position values " + f"(one per order_by key), got {len(bound)}" + ) + return _after_query_values(bound) + + +def _compile_after(query: "Query[Any]") -> tuple[QueryValue, ...] | None: + """Serialize a fetch payload's ``after`` bound, re-checking current order keys.""" + return _compile_position(query, query._after, op="after") + + +def _compile_before(query: "Query[Any]") -> tuple[QueryValue, ...] | None: + """Serialize a fetch payload's ``before`` bound, re-checking current order keys.""" + return _compile_position(query, query._before, op="before") + + 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()``/``before()`` 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 + or query._before 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/before: portable SQL " + f"has no {operation.upper()} ... LIMIT. Remove the " + f".limit()/.offset()/.after()/.before() call, or fetch primary keys first " + f"and {operation} by primary-key set." ) if ( query._joins @@ -892,12 +939,14 @@ def compile_query( What each verb carries is policy, stated here once: - - ``fetch`` carries everything: ordering, paging, m2m context, joins, + - ``fetch`` carries everything: ordering, paging (``limit``/``offset`` + keys, plus optional ``after`` / ``before`` bounds), 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`` / ``before`` 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. @@ -934,6 +983,8 @@ def compile_query( order_by=(), limit=_ABSENT, offset=_ABSENT, + after=None, + before=None, m2m=None, joins=(), materialization=RootInstances(), @@ -946,6 +997,8 @@ def compile_query( order_by=(), limit=None, offset=None, + after=None, + before=None, m2m=query._m2m_context, joins=_serialize_joins(query), materialization=RootInstances(), @@ -958,6 +1011,8 @@ def compile_query( order_by=tuple(query.order_by_clause), limit=query._limit, offset=query._offset, + after=_compile_after(query), + before=_compile_before(query), m2m=query._m2m_context, joins=_serialize_joins(query), materialization=_materialization(query), diff --git a/src/operations.rs b/src/operations.rs index 2f1b7c9..966f366 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -252,12 +252,12 @@ fn tx_remove(session_id: Option<&str>, tx_id: &str) -> PyResult { + match order.nulls.to_lowercase().as_str() { + "native" => { select.order_by_expr(col, dir); Ok(()) } - Some("first") => { + "first" => { select.order_by_expr_with_nulls(col, dir, NullOrdering::First); Ok(()) } - Some("last") => { + "last" => { select.order_by_expr_with_nulls(col, dir, NullOrdering::Last); Ok(()) } - Some(other) => Err(pyo3::exceptions::PyValueError::new_err(format!( - "invalid order_by nulls {other:?}: expected \"first\" or \"last\"" + other => Err(pyo3::exceptions::PyValueError::new_err(format!( + "invalid order_by nulls {other:?}: expected \"first\", \"last\", or \"native\"" ))), } } @@ -1218,12 +1218,17 @@ fn materialize_model_row<'py>( Ok(instance) } -/// Enforce the v8 verb contract for `limit`/`offset` key presence. +/// Enforce the v14 verb contract for paging-key presence (#393/#395). 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() + || plan.before.is_some() => + { Err(pyo3::exceptions::PyValueError::new_err(format!( - "{operation} QueryIR must omit limit/offset keys" + "{operation} QueryIR must omit limit/offset/after/before keys" ))) } "fetch" | "count" if plan.limit.is_none() || plan.offset.is_none() => { @@ -1231,6 +1236,24 @@ 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() || plan.before.is_some() => { + Err(pyo3::exceptions::PyValueError::new_err( + "count QueryIR must omit after/before keys (paging is dropped)".to_string(), + )) + } + "fetch" if plan.after.is_some() && plan.before.is_some() => { + Err(pyo3::exceptions::PyValueError::new_err( + "after cannot be combined with before: a query has one start".to_string(), + )) + } + "fetch" + if (plan.after.is_some() || plan.before.is_some()) + && plan.offset.flatten().is_some() => + { + Err(pyo3::exceptions::PyValueError::new_err( + "after/before 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 +2802,21 @@ 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); + } + if let Some(before) = plan + .before_condition(backend, &table_name, &join_plan) + .map_err(pyo3::exceptions::PyValueError::new_err)? + { + condition = condition.add(before); + } + 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 @@ -2792,7 +2824,12 @@ pub fn fetch_filtered<'py>( // this is defense-in-depth, never reachable in normal use. On a // record plan the term resolves output field names FIRST (#295): // a matching name renders as the bare result-column alias. - for order in &plan.order_by { + // `before` inverts direction and first↔last (`native` stays + // `native`) so LIMIT n fetches the adjacent previous page (#395). + let order_by = plan + .order_by_for_select() + .map_err(pyo3::exceptions::PyValueError::new_err)?; + for order in &order_by { let col = match projected.as_deref() { Some(fields) => record_order_by_expr(order, fields, &table_name, &join_plan)?, None => crate::query::qualify_column_with_joins( @@ -5388,7 +5425,7 @@ mod mutation_pagination_guard_tests { fn envelope_without_pagination_keys() -> String { serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Widget", @@ -5576,10 +5613,10 @@ mod mutation_pagination_guard_tests { mod query_ir_version_gate_tests { use super::query_plan_from_ir_json; - fn v11_envelope() -> serde_json::Value { + fn v14_envelope() -> serde_json::Value { serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Widget", @@ -5597,7 +5634,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), and v11 (#379). + /// v4 (#285), v5 (#292), v6 (#310), v7 (#314), v8 (#376), v9 (#377), v10 (#378), v11 (#379), v12 (#392), v13 (#393), and v14 (#395). fn assert_actionable_version_rejection(err: pyo3::PyErr, received: char) { pyo3::Python::attach(|py| { assert!(err.is_instance_of::(py)); @@ -5608,7 +5645,7 @@ mod query_ir_version_gate_tests { "message should name the received version: {msg}" ); assert!( - msg.contains("11"), + msg.contains("14"), "message should name the supported version: {msg}" ); assert!( @@ -5622,21 +5659,21 @@ mod query_ir_version_gate_tests { } #[test] - fn accepts_version_11() { - query_plan_from_ir_json(&v11_envelope().to_string()) - .expect("a well-formed v11 envelope must be accepted"); + fn accepts_version_14() { + query_plan_from_ir_json(&v14_envelope().to_string()) + .expect("a well-formed v14 envelope must be accepted"); } #[test] fn rejects_v8_payload_without_set_section() { - let mut envelope = v11_envelope(); + let mut envelope = v14_envelope(); envelope["payload"] .as_object_mut() .expect("payload object") .remove("set"); let err = query_plan_from_ir_json(&envelope.to_string()) - .expect_err("a v11 payload without its required SET section must be rejected"); + .expect_err("a v14 payload without its required SET section must be rejected"); assert!( err.to_string().contains("set"), "must name the missing section: {err}" @@ -5645,7 +5682,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v7_envelope_with_actionable_message() { - let mut envelope = v11_envelope(); + let mut envelope = v14_envelope(); envelope["ir_version"] = serde_json::json!(7); envelope["payload"] .as_object_mut() @@ -5659,7 +5696,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v8_envelope_with_actionable_message() { - let mut envelope = v11_envelope(); + let mut envelope = v14_envelope(); envelope["ir_version"] = serde_json::json!(8); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5669,7 +5706,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v9_envelope_with_actionable_message() { - let mut envelope = v11_envelope(); + let mut envelope = v14_envelope(); envelope["ir_version"] = serde_json::json!(9); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5679,7 +5716,7 @@ mod query_ir_version_gate_tests { #[test] fn rejects_v10_envelope_with_actionable_message() { - let mut envelope = v11_envelope(); + let mut envelope = v14_envelope(); envelope["ir_version"] = serde_json::json!(10); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5689,8 +5726,74 @@ mod query_ir_version_gate_tests { msg.contains("10"), "message should name the received version: {msg}" ); + assert!( + msg.contains("14"), + "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_v11_envelope_with_actionable_message() { + let mut envelope = v14_envelope(); + envelope["ir_version"] = serde_json::json!(11); + + let err = query_plan_from_ir_json(&envelope.to_string()) + .expect_err("a v11 envelope must be rejected before payload parsing"); + let msg = err.to_string(); assert!( msg.contains("11"), + "message should name the received version: {msg}" + ); + assert!( + msg.contains("14"), + "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 = v14_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("14"), + "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_v13_envelope_with_actionable_message() { + let mut envelope = v14_envelope(); + envelope["ir_version"] = serde_json::json!(13); + + let err = query_plan_from_ir_json(&envelope.to_string()) + .expect_err("a v13 envelope must be rejected before payload parsing"); + let msg = err.to_string(); + assert!( + msg.contains("13"), + "message should name the received version: {msg}" + ); + assert!( + msg.contains("14"), "message should name the supported version: {msg}" ); assert!( @@ -5882,14 +5985,14 @@ mod query_ir_version_gate_tests { #[test] fn rejects_unsupported_future_version() { - let mut envelope = v11_envelope(); - envelope["ir_version"] = serde_json::json!(12); + let mut envelope = v14_envelope(); + envelope["ir_version"] = serde_json::json!(15); 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("12"), + msg.contains("15"), "message should name the received version: {msg}" ); } @@ -5898,7 +6001,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 = v11_envelope(); + let mut envelope = v14_envelope(); envelope["payload"]["materialization"] = serde_json::json!({"kind": "row_dicts"}); let err = query_plan_from_ir_json(&envelope.to_string()) @@ -5915,7 +6018,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 = v11_envelope(); + let mut envelope = v14_envelope(); envelope["payload"] .as_object_mut() .unwrap() @@ -5942,7 +6045,7 @@ mod materialization_walker_gate_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Widget", @@ -6035,7 +6138,7 @@ mod record_select_list_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Transaction", @@ -6418,7 +6521,7 @@ mod record_select_list_tests { column: "total".to_string(), direction: "desc".to_string(), path: vec![], - nulls: None, + nulls: "last".to_string(), }; let expr = record_order_by_expr(&order, &projected, "transaction", &JoinPlan::default()) .expect("output-name term resolves"); @@ -6449,7 +6552,7 @@ mod record_select_list_tests { column: "account_id".to_string(), direction: "asc".to_string(), path: vec![], - nulls: None, + nulls: "last".to_string(), }; let expr = record_order_by_expr(&order, &projected, "transaction", &JoinPlan::default()) .expect("group-key source term resolves"); @@ -6482,7 +6585,7 @@ mod record_select_list_tests { column: "note".to_string(), direction: "asc".to_string(), path: vec![], - nulls: None, + nulls: "last".to_string(), }; let err = record_order_by_expr(&order, &projected, "transaction", &JoinPlan::default()) .expect_err("an ungrouped bare term must be rejected"); @@ -6538,7 +6641,7 @@ mod instances_select_list_tests { let mut plan = query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Transaction", @@ -6698,7 +6801,7 @@ mod mutation_qualification_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Widget", @@ -6785,7 +6888,7 @@ mod select_join_render_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Transaction", @@ -6796,7 +6899,7 @@ mod select_join_render_tests { "value": {"kind": "string", "value": "a1"}, "path": ["account"] }], - "order_by": [{"column": "name", "direction": "asc", "path": ["account"]}], + "order_by": [{"column": "name", "direction": "asc", "path": ["account"], "nulls": "last"}], "limit": null, "offset": null, "m2m": null, "materialization": {"kind": "root_instances"}, "joins": [ {"join_type": "inner", "path": [ @@ -6892,7 +6995,7 @@ mod select_join_render_tests { query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": model_name, @@ -6915,7 +7018,7 @@ mod select_join_render_tests { let plan = query_plan_from_ir_json( &serde_json::json!({ "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "set": [], "model_name": "Transaction", @@ -7060,22 +7163,18 @@ mod select_join_render_tests { #[cfg(test)] mod order_by_nulls_render_tests { - //! #361: optional `order_by[].nulls` renders native NULLS FIRST/LAST on - //! both Postgres and SQLite via sea-query. Absent key keeps plain ASC/DESC. + //! #392: required `order_by[].nulls` renders native NULLS FIRST/LAST or + //! plain ASC/DESC when `native`. Missing key fails at IR decode. use super::apply_order_by_term; use sea_query::{Alias, Expr, PostgresQueryBuilder, Query, SqliteQueryBuilder}; - fn order_term( - column: &str, - direction: &str, - nulls: Option<&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(), path: vec![], - nulls: nulls.map(str::to_string), + nulls: nulls.to_string(), } } @@ -7098,7 +7197,7 @@ mod order_by_nulls_render_tests { #[test] fn nulls_last_with_desc_emits_native_clause_on_both_dialects() { - let order = order_term("pinned_at", "desc", Some("last")); + let order = order_term("pinned_at", "desc", "last"); for (postgres, label) in [(true, "postgres"), (false, "sqlite")] { let sql = render_order_sql(&order, postgres).expect(label); assert!( @@ -7114,7 +7213,7 @@ mod order_by_nulls_render_tests { #[test] fn nulls_first_emits_native_clause_on_both_dialects() { - let order = order_term("pinned_at", "asc", Some("first")); + let order = order_term("pinned_at", "asc", "first"); for (postgres, label) in [(true, "postgres"), (false, "sqlite")] { let sql = render_order_sql(&order, postgres).expect(label); assert!( @@ -7125,45 +7224,53 @@ mod order_by_nulls_render_tests { } #[test] - fn absent_nulls_keeps_plain_direction_without_nulls_clause() { - let order = order_term("pinned_at", "desc", None); + fn native_nulls_keeps_plain_direction_without_nulls_clause() { + let order = order_term("pinned_at", "desc", "native"); for (postgres, label) in [(true, "postgres"), (false, "sqlite")] { let sql = render_order_sql(&order, postgres).expect(label); assert!(sql.contains("DESC"), "{label} must still emit DESC: {sql}"); assert!( !sql.contains("NULLS"), - "{label} must omit NULLS when unset: {sql}" + "{label} must omit NULLS for native: {sql}" ); } } #[test] fn nulls_token_is_case_insensitive() { - for token in ["LAST", "Last", "FIRST", "First"] { - let order = order_term("pinned_at", "desc", Some(token)); + for token in ["LAST", "Last", "FIRST", "First", "NATIVE", "Native"] { + let order = order_term("pinned_at", "desc", token); let sql = render_order_sql(&order, true).unwrap_or_else(|e| panic!("{token}: {e}")); let expected = if token.eq_ignore_ascii_case("last") { "NULLS LAST" - } else { + } else if token.eq_ignore_ascii_case("first") { "NULLS FIRST" + } else { + continue; }; assert!( sql.contains(expected), "case-insensitive {token:?} must emit {expected}: {sql}" ); } + let native = order_term("pinned_at", "desc", "NATIVE"); + let sql = render_order_sql(&native, true).expect("native"); + assert!( + !sql.contains("NULLS"), + "case-insensitive NATIVE must omit NULLS clause: {sql}" + ); } #[test] fn junk_nulls_errors_loudly_not_silently() { - let order = order_term("pinned_at", "desc", Some("sideways")); + let order = order_term("pinned_at", "desc", "sideways"); let err = render_order_sql(&order, true).expect_err("junk nulls must fail"); assert!( err.contains("sideways"), "error must name the bad value: {err}" ); assert!( - err.contains("first") && err.contains("last"), + err.contains("first") && err.contains("last") && err.contains("native"), "error must name the accepted tokens: {err}" ); } diff --git a/src/query.rs b/src/query.rs index e127dd2..aab2534 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,157 @@ pub fn qualify_column_with_joins( qualify_leaf_column(&qualifier, column, path) } +/// One term of an exclusive stepwise (keyset) compare (#393/#394). +/// +/// Keep compare logic here, not in the SELECT builder. `nulls` is the wire +/// token (`"last"` / `"first"` / `"native"`); `"native"` resolves from +/// [`Dialect`] inside [`exclusive_stepwise_compare`]. +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, + /// Wire `order_by[].nulls` token (`"last"` / `"first"` / `"native"`). + pub nulls: String, + /// Whether this slot's bound is SQL NULL (`kind: "null"`). + pub bound_is_null: bool, +} + +/// Resolved NULLS placement after `"native"` is lowered for one dialect. +#[derive(Clone, Copy, PartialEq, Eq)] +enum NullsPlacement { + First, + Last, +} + +/// 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 `<`. NULL placement and a NULL bound fold into the same tree so a +/// cursor can cross the NULL bucket in one query. `"native"` resolves here +/// from `dialect` (Postgres: NULL is larger; SQLite: NULL is smaller). +pub fn exclusive_stepwise_compare( + keys: &[StepwiseKey], + dialect: Dialect, +) -> Result { + if keys.is_empty() { + return Err("exclusive stepwise compare requires at least one order key".to_string()); + } + let mut any = Condition::any(); + let mut added = false; + for i in 0..keys.len() { + let Some(after_term) = stepwise_after_term(&keys[i], dialect)? else { + continue; + }; + let mut prefix = Condition::all(); + for key in &keys[..i] { + prefix = prefix.add(stepwise_eq(key)); + } + prefix = prefix.add(after_term); + any = any.add(prefix); + added = true; + } + if !added { + return Ok(Condition::all().add(Expr::cust("FALSE"))); + } + Ok(any) +} + +fn resolve_nulls_placement( + nulls: &str, + direction: &str, + dialect: Dialect, +) -> Result { + match nulls.to_ascii_lowercase().as_str() { + "first" => Ok(NullsPlacement::First), + "last" => Ok(NullsPlacement::Last), + "native" => { + let desc = direction.to_ascii_lowercase() == "desc"; + Ok(match (dialect, desc) { + (Dialect::Postgres, false) => NullsPlacement::Last, + (Dialect::Postgres, true) => NullsPlacement::First, + (Dialect::Sqlite, false) => NullsPlacement::First, + (Dialect::Sqlite, true) => NullsPlacement::Last, + }) + } + other => Err(format!( + "invalid order_by nulls {other:?}: expected \"first\", \"last\", or \"native\"" + )), + } +} + +fn stepwise_eq(key: &StepwiseKey) -> SimpleExpr { + if key.bound_is_null { + key.column.clone().is_null() + } else { + key.column.clone().eq(key.bound.clone()) + } +} + +fn stepwise_after_term(key: &StepwiseKey, dialect: Dialect) -> Result, String> { + let placement = resolve_nulls_placement(&key.nulls, &key.direction, dialect)?; + if key.bound_is_null { + return Ok(match placement { + NullsPlacement::Last => None, + NullsPlacement::First => Some(Condition::all().add(key.column.clone().is_not_null())), + }); + } + let mut term = Condition::any(); + term = term.add(stepwise_inequality(key)?); + if placement == NullsPlacement::Last { + term = term.add(key.column.clone().is_null()); + } + Ok(Some(term)) +} + +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\"" + )), + } +} + +/// Invert one order key for `before()` (#395): `asc`↔`desc`, `first`↔`last`, +/// `"native"` stays `"native"` (it resolves from [`Dialect`] at render). +pub fn invert_order_key(direction: &str, nulls: &str) -> Result<(String, String), String> { + let direction = match direction.to_ascii_lowercase().as_str() { + "asc" => "desc".to_string(), + "desc" => "asc".to_string(), + other => { + return Err(format!( + "invalid order_by direction {other:?}: expected \"asc\" or \"desc\"" + )); + } + }; + let nulls = match nulls.to_ascii_lowercase().as_str() { + "first" => "last".to_string(), + "last" => "first".to_string(), + "native" => "native".to_string(), + other => { + return Err(format!( + "invalid order_by nulls {other:?}: expected \"first\", \"last\", or \"native\"" + )); + } + }; + Ok((direction, nulls)) +} + +/// Invert a wire `order_by` term the same way [`invert_order_key`] does. +pub fn invert_order_by(order: &QueryOrderBy) -> Result { + let (direction, nulls) = invert_order_key(&order.direction, &order.nulls)?; + Ok(QueryOrderBy { + column: order.column.clone(), + direction, + path: order.path.clone(), + nulls, + }) +} + /// One rendered relation JOIN edge: ` AS ON /// . = .`. #[derive(Debug, Clone)] @@ -207,7 +359,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 +499,12 @@ 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>, + /// Exclusive previous-page bound (`None` = omitted). Fetch only; count + /// and mutations omit it (#395). Combined with `after` is a loud error. + pub before: Option>, /// Relation JOINs collected from WHERE traversal, in registration order. /// Empty for non-traversal queries; rendered by the SELECT walkers (#270). pub joins: Vec, @@ -400,6 +559,9 @@ impl QueryPlan { .map_err(|e| format!("invalid QueryIR m2m payload: {e}"))?, None => None, }; + if payload.after.is_some() && payload.before.is_some() { + return Err("after cannot be combined with before: a query has one start".to_string()); + } let registration = crate::state::MODEL_REGISTRY .read() .ok() @@ -411,6 +573,8 @@ impl QueryPlan { order_by: payload.order_by, limit: payload.limit, offset: payload.offset, + after: payload.after, + before: payload.before, joins: payload.joins, m2m, materialization: payload.materialization, @@ -429,7 +593,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 +633,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 +728,91 @@ 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`] — this walker only qualifies, binds, + /// and passes each key's ``nulls`` token plus a NULL-bound fact. + pub fn after_condition( + &self, + backend: Dialect, + root_table: &str, + join_plan: &JoinPlan, + ) -> Result, String> { + self.position_condition(self.after.as_deref(), false, backend, root_table, join_plan) + } + + /// Bind a ``before`` bound as ``after`` on inverted order keys (#395). + /// + /// Invert happens here; [`exclusive_stepwise_compare`] is the only expander. + pub fn before_condition( + &self, + backend: Dialect, + root_table: &str, + join_plan: &JoinPlan, + ) -> Result, String> { + self.position_condition(self.before.as_deref(), true, backend, root_table, join_plan) + } + + /// `ORDER BY` terms for the SELECT: inverted when ``before`` is set so + /// `LIMIT n` fetches the adjacent previous page. + pub fn order_by_for_select(&self) -> Result, String> { + if self.before.is_some() { + self.order_by.iter().map(invert_order_by).collect() + } else { + Ok(self.order_by.clone()) + } + } + + fn position_condition( + &self, + values: Option<&[QueryValue]>, + invert: bool, + backend: Dialect, + root_table: &str, + join_plan: &JoinPlan, + ) -> Result, String> { + if self.after.is_some() && self.before.is_some() { + return Err("after cannot be combined with before: a query has one start".to_string()); + } + let Some(values) = values else { + return Ok(None); + }; + let start = if invert { "before" } else { "after" }; + if self.offset.flatten().is_some() { + return Err(format!( + "{start} cannot be combined with offset: a query has one start" + )); + } + if values.len() != self.order_by.len() { + return Err(format!( + "{start} 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 (direction, nulls) = if invert { + invert_order_key(&order.direction, &order.nulls)? + } else { + (order.direction.clone(), order.nulls.clone()) + }; + 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, + bound, + nulls, + bound_is_null: value.kind.eq_ignore_ascii_case("null") || value.value.is_null(), + }); + } + Ok(Some(exclusive_stepwise_compare(&keys, backend)?)) + } + fn build_condition( &self, backend: Dialect, @@ -854,7 +1102,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 +1177,11 @@ impl QueryPlan { #[cfg(test)] mod tests { - use super::QueryPlan; + use super::{exclusive_stepwise_compare, JoinPlan, QueryPlan, StepwiseKey}; 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 +1197,8 @@ mod tests { order_by: Vec::new(), limit: Some(None), offset: Some(None), + after: None, + before: None, joins: Vec::new(), m2m: None, materialization: ferro_schema_ir::Materialization::RootInstances, @@ -960,6 +1218,454 @@ mod tests { values.0.into_iter().next().expect("one value") } + fn render_stepwise(keys: &[StepwiseKey]) -> String { + let cond = exclusive_stepwise_compare(keys, Dialect::Sqlite).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(), + nulls: "last".into(), + bound_is_null: false, + }, + StepwiseKey { + column: Expr::col(Alias::new("b")), + direction: "asc".into(), + bound: Expr::val(2).into(), + nulls: "last".into(), + bound_is_null: false, + }, + ]); + 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(), + nulls: "last".into(), + bound_is_null: false, + }, + StepwiseKey { + column: Expr::col(Alias::new("b")), + direction: "desc".into(), + bound: Expr::val(2).into(), + nulls: "last".into(), + bound_is_null: false, + }, + ]); + 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}" + ); + } + + fn after_plan(order: &[(&str, &str, &str)], after: serde_json::Value) -> QueryPlan { + let order_by: Vec = order + .iter() + .map(|(column, direction, nulls)| { + json!({ + "column": column, + "direction": direction, + "path": [], + "nulls": nulls, + }) + }) + .collect(); + let payload: ferro_schema_ir::QueryIrPayload = serde_json::from_value(json!({ + "set": [], + "model_name": "Pending", + "where": [], + "order_by": order_by, + "limit": null, + "offset": null, + "after": after, + "m2m": null, + "materialization": {"kind": "root_instances"}, + "joins": [] + })) + .expect("after payload deserializes"); + QueryPlan::from_ir_payload(payload).expect("plan builds") + } + + fn render_after( + order: &[(&str, &str, &str)], + after: serde_json::Value, + dialect: Dialect, + ) -> String { + let plan = after_plan(order, after); + let cond = plan + .after_condition(dialect, "t", &JoinPlan::default()) + .expect("after_condition") + .expect("after present"); + let mut select = Query::select(); + select + .column(Alias::new("id")) + .from(Alias::new("t")) + .cond_where(cond); + match dialect { + Dialect::Sqlite => select.to_string(SqliteQueryBuilder), + Dialect::Postgres => select.to_string(PostgresQueryBuilder), + } + } + + fn flatten_sql(sql: &str) -> String { + sql.replace('"', "").replace('`', "").to_lowercase() + } + + #[test] + fn after_asc_last_non_null_includes_later_null_bucket() { + let sql = render_after( + &[("a", "asc", "last"), ("b", "asc", "last")], + json!([{"kind": "int", "value": 1}, {"kind": "int", "value": 2}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("a > 1") && flat.contains("a is null") && flat.contains("a = 1"), + "ASC last after non-NULL must cross into the NULL bucket: {sql}" + ); + assert!( + flat.contains("b > 2"), + "prefix-equal arm must keep the next-key inequality: {sql}" + ); + } + + #[test] + fn after_desc_last_pinch_non_null_crosses_into_null_bucket() { + let sql = render_after( + &[("pinned_at", "desc", "last"), ("id", "asc", "last")], + json!([{"kind": "int", "value": 10}, {"kind": "int", "value": 3}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("pinned_at < 10") && flat.contains("pinned_at is null"), + "DESC last after a pinned value must include unpinned: {sql}" + ); + assert!( + flat.contains("pinned_at = 10") && flat.contains("id > 3"), + "same-pin prefix must continue by PK: {sql}" + ); + } + + #[test] + fn after_desc_last_pinch_null_bound_stays_in_null_bucket() { + let sql = render_after( + &[("pinned_at", "desc", "last"), ("id", "asc", "last")], + json!([{"kind": "null", "value": null}, {"kind": "int", "value": 4}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("pinned_at is null") && flat.contains("id > 4"), + "after((None, id)) must walk remaining NULLs by PK: {sql}" + ); + assert!( + !flat.contains("pinned_at is not null"), + "NULLS LAST has nothing after the NULL bucket: {sql}" + ); + assert!( + !flat.contains("pinned_at <") && !flat.contains("pinned_at >"), + "NULL bound must not emit an inequality on the leading key: {sql}" + ); + } + + #[test] + fn after_asc_first_null_bound_crosses_into_non_null_bucket() { + let sql = render_after( + &[("a", "asc", "first"), ("b", "asc", "last")], + json!([{"kind": "null", "value": null}, {"kind": "int", "value": 2}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("a is not null"), + "ASC first after NULL must include the later non-NULL bucket: {sql}" + ); + assert!( + flat.contains("a is null") && flat.contains("b > 2"), + "remaining NULL-bucket rows continue by the next key: {sql}" + ); + } + + #[test] + fn after_asc_first_non_null_does_not_reenter_null_bucket() { + let sql = render_after( + &[("a", "asc", "first"), ("b", "asc", "last")], + json!([{"kind": "int", "value": 1}, {"kind": "int", "value": 2}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("a > 1") && flat.contains("a = 1") && flat.contains("b > 2"), + "ASC first after non-NULL keeps the exclusive compare: {sql}" + ); + assert!( + !flat.contains("a is null"), + "NULLs already precede the cursor: {sql}" + ); + } + + #[test] + fn after_desc_first_non_null_does_not_reenter_null_bucket() { + let sql = render_after( + &[("a", "desc", "first"), ("b", "desc", "last")], + json!([{"kind": "int", "value": 1}, {"kind": "int", "value": 2}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("a < 1") && flat.contains("a = 1") && flat.contains("b < 2"), + "DESC first after non-NULL keeps the exclusive compare: {sql}" + ); + assert!( + !flat.contains("a is null"), + "NULLs already precede the cursor: {sql}" + ); + } + + #[test] + fn after_desc_first_null_bound_crosses_into_non_null_bucket() { + let sql = render_after( + &[("a", "desc", "first"), ("b", "asc", "last")], + json!([{"kind": "null", "value": null}, {"kind": "int", "value": 2}]), + Dialect::Sqlite, + ); + let flat = flatten_sql(&sql); + assert!( + flat.contains("a is not null"), + "DESC first after NULL must include later non-NULL values: {sql}" + ); + assert!( + flat.contains("a is null") && flat.contains("b > 2"), + "remaining NULL-bucket rows continue by the next key: {sql}" + ); + } + + #[test] + fn after_native_resolves_at_render_from_dialect() { + let order = &[("a", "asc", "native"), ("b", "asc", "last")]; + let after = json!([{"kind": "int", "value": 1}, {"kind": "int", "value": 2}]); + let pg = flatten_sql(&render_after(order, after.clone(), Dialect::Postgres)); + let sqlite = flatten_sql(&render_after(order, after.clone(), Dialect::Sqlite)); + assert!( + pg.contains("a is null"), + "Postgres ASC native treats NULL as larger (last): {pg}" + ); + assert!( + !sqlite.contains("a is null"), + "SQLite ASC native treats NULL as smaller (first): {sqlite}" + ); + + let desc = &[("a", "desc", "native"), ("b", "asc", "last")]; + let pg_desc = flatten_sql(&render_after(desc, after.clone(), Dialect::Postgres)); + let sqlite_desc = flatten_sql(&render_after(desc, after, Dialect::Sqlite)); + assert!( + !pg_desc.contains("a is null"), + "Postgres DESC native treats NULL as larger (first): {pg_desc}" + ); + assert!( + sqlite_desc.contains("a is null"), + "SQLite DESC native treats NULL as smaller (last): {sqlite_desc}" + ); + } + + #[test] + fn invert_order_key_swaps_direction_and_first_last_keeps_native() { + assert_eq!( + super::invert_order_key("asc", "last").expect("invert"), + ("desc".into(), "first".into()) + ); + assert_eq!( + super::invert_order_key("desc", "first").expect("invert"), + ("asc".into(), "last".into()) + ); + assert_eq!( + super::invert_order_key("ASC", "NATIVE").expect("invert"), + ("desc".into(), "native".into()) + ); + } + + fn before_plan(order: &[(&str, &str, &str)], before: serde_json::Value) -> QueryPlan { + let order_by: Vec = order + .iter() + .map(|(column, direction, nulls)| { + json!({ + "column": column, + "direction": direction, + "path": [], + "nulls": nulls, + }) + }) + .collect(); + let payload: ferro_schema_ir::QueryIrPayload = serde_json::from_value(json!({ + "set": [], + "model_name": "Pending", + "where": [], + "order_by": order_by, + "limit": null, + "offset": null, + "before": before, + "m2m": null, + "materialization": {"kind": "root_instances"}, + "joins": [] + })) + .expect("before payload deserializes"); + QueryPlan::from_ir_payload(payload).expect("plan builds") + } + + fn render_before( + order: &[(&str, &str, &str)], + before: serde_json::Value, + dialect: Dialect, + ) -> String { + let plan = before_plan(order, before); + let cond = plan + .before_condition(dialect, "t", &JoinPlan::default()) + .expect("before_condition") + .expect("before present"); + let mut select = Query::select(); + select + .column(Alias::new("id")) + .from(Alias::new("t")) + .cond_where(cond); + match dialect { + Dialect::Sqlite => select.to_string(SqliteQueryBuilder), + Dialect::Postgres => select.to_string(PostgresQueryBuilder), + } + } + + #[test] + fn before_is_after_on_inverted_keys_same_expander() { + let order = &[("pinned_at", "desc", "last"), ("id", "asc", "last")]; + let inverted = &[("pinned_at", "asc", "first"), ("id", "desc", "first")]; + let bound = json!([{"kind": "null", "value": null}, {"kind": "int", "value": 4}]); + let before_sql = flatten_sql(&render_before(order, bound.clone(), Dialect::Sqlite)); + let after_sql = flatten_sql(&render_after(inverted, bound, Dialect::Sqlite)); + assert_eq!( + before_sql, after_sql, + "before must reuse exclusive_stepwise_compare via inverted keys" + ); + } + + #[test] + fn before_native_stays_native_when_inverting() { + let order = &[("a", "asc", "native"), ("b", "asc", "last")]; + let inverted = &[("a", "desc", "native"), ("b", "desc", "first")]; + let bound = json!([{"kind": "int", "value": 1}, {"kind": "int", "value": 2}]); + let before_pg = flatten_sql(&render_before(order, bound.clone(), Dialect::Postgres)); + let after_pg = flatten_sql(&render_after(inverted, bound.clone(), Dialect::Postgres)); + assert_eq!(before_pg, after_pg, "native must stay native: {before_pg}"); + let before_sqlite = flatten_sql(&render_before(order, bound.clone(), Dialect::Sqlite)); + let after_sqlite = flatten_sql(&render_after(inverted, bound, Dialect::Sqlite)); + assert_eq!( + before_sqlite, after_sqlite, + "native must stay native on sqlite: {before_sqlite}" + ); + } + + #[test] + fn before_condition_qualifies_path_carrying_order_key() { + // before inverts keys then reuses exclusive_stepwise_compare; the + // column still goes through qualify_column_with_joins (#396). + let payload: ferro_schema_ir::QueryIrPayload = serde_json::from_value(json!({ + "set": [], + "model_name": "Transaction", + "where": [], + "order_by": [ + {"column": "label", "direction": "asc", "path": ["account"], "nulls": "last"}, + {"column": "id", "direction": "asc", "path": [], "nulls": "last"} + ], + "limit": null, + "offset": null, + "before": [ + {"kind": "string", "value": "a1"}, + {"kind": "int", "value": 2} + ], + "m2m": null, + "materialization": {"kind": "root_instances"}, + "joins": [ + {"join_type": "inner", "path": [ + {"relation": "account", "from_column": "account_id", + "to_table": "account", "to_column": "id"} + ]} + ] + })) + .expect("payload deserializes"); + let plan = QueryPlan::from_ir_payload(payload).expect("plan builds"); + let join_plan = plan.build_join_plan("transaction", &[]).expect("join plan"); + let cond = plan + .before_condition(Dialect::Sqlite, "transaction", &join_plan) + .expect("before_condition") + .expect("before present"); + let mut select = Query::select(); + select + .column(Alias::new("id")) + .from(Alias::new("transaction")) + .cond_where(cond); + let sql = flatten_sql(&select.to_string(SqliteQueryBuilder)); + assert!( + sql.contains("j1_account.label"), + "before+path must qualify the traversed key by its join alias: {sql}" + ); + assert!( + sql.contains("transaction.id"), + "root PK key stays on the root table: {sql}" + ); + } + + #[test] + fn after_and_before_together_fail_loud() { + let payload: ferro_schema_ir::QueryIrPayload = serde_json::from_value(json!({ + "set": [], + "model_name": "Pending", + "where": [], + "order_by": [ + {"column": "id", "direction": "asc", "path": [], "nulls": "last"} + ], + "limit": null, + "offset": null, + "after": [{"kind": "int", "value": 1}], + "before": [{"kind": "int", "value": 2}], + "m2m": null, + "materialization": {"kind": "root_instances"}, + "joins": [] + })) + .expect("payload with both bounds deserializes"); + let err = QueryPlan::from_ir_payload(payload).expect_err("after + before must fail"); + assert!( + err.contains("after") && err.contains("before"), + "error must name both starts: {err}" + ); + } + #[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!({ @@ -974,7 +1680,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"); @@ -1497,7 +2203,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": [ @@ -1521,7 +2227,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 +2261,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 +2518,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 +2549,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] @@ -1916,7 +2639,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"); @@ -1932,13 +2655,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 +2750,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 +2778,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 +2806,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 +2834,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 +2889,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 +2921,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 e788e6f..196cb2a 100644 --- a/tests/fixtures/ir_vectors/README.md +++ b/tests/fixtures/ir_vectors/README.md @@ -28,8 +28,11 @@ 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: 11` (#379 — unconditional bump; every payload - carries a required canonical `set` list). v7 introduced the recursive + vectors are on `ir_version: 14` (#395 — optional `before` position bound on + fetch payloads; omitted when unset; `after` from v13 still 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_v11.json b/tests/fixtures/ir_vectors/query_account_exists_v14.json similarity index 86% rename from tests/fixtures/ir_vectors/query_account_exists_v11.json rename to tests/fixtures/ir_vectors/query_account_exists_v14.json index e8c4c7c..159027e 100644 --- a/tests/fixtures/ir_vectors/query_account_exists_v11.json +++ b/tests/fixtures/ir_vectors/query_account_exists_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_account_exists_v11", + "vector_name": "query_account_exists_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Account", "where": [ @@ -26,7 +26,8 @@ { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": null, diff --git a/tests/fixtures/ir_vectors/query_account_scoped_exists_v11.json b/tests/fixtures/ir_vectors/query_account_scoped_exists_v14.json similarity index 95% rename from tests/fixtures/ir_vectors/query_account_scoped_exists_v11.json rename to tests/fixtures/ir_vectors/query_account_scoped_exists_v14.json index 63be422..e29d2d6 100644 --- a/tests/fixtures/ir_vectors/query_account_scoped_exists_v11.json +++ b/tests/fixtures/ir_vectors/query_account_scoped_exists_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_account_scoped_exists_v11", + "vector_name": "query_account_scoped_exists_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Account", "where": [ diff --git a/tests/fixtures/ir_vectors/query_card_after_null_slot_v14.json b/tests/fixtures/ir_vectors/query_card_after_null_slot_v14.json new file mode 100644 index 0000000..82bac4b --- /dev/null +++ b/tests/fixtures/ir_vectors/query_card_after_null_slot_v14.json @@ -0,0 +1,56 @@ +{ + "vector_name": "query_card_after_null_slot_v14", + "domain": "query", + "expect_valid": true, + "ir": { + "ir_kind": "query", + "ir_version": 14, + "payload": { + "model_name": "Card", + "where": [ + { + "node_kind": "leaf", + "column": "id", + "operator": "!=", + "value": { + "kind": "null", + "value": null + }, + "path": [] + } + ], + "set": [], + "order_by": [ + { + "column": "pinned_at", + "direction": "desc", + "path": [], + "nulls": "last" + }, + { + "column": "id", + "direction": "asc", + "path": [], + "nulls": "last" + } + ], + "limit": null, + "offset": null, + "after": [ + { + "kind": "null", + "value": null + }, + { + "kind": "int", + "value": 4 + } + ], + "m2m": null, + "joins": [], + "materialization": { + "kind": "root_instances" + } + } + } +} diff --git a/tests/fixtures/ir_vectors/query_card_nulls_v11.json b/tests/fixtures/ir_vectors/query_card_nulls_v14.json similarity index 87% rename from tests/fixtures/ir_vectors/query_card_nulls_v11.json rename to tests/fixtures/ir_vectors/query_card_nulls_v14.json index fecf40f..01d6467 100644 --- a/tests/fixtures/ir_vectors/query_card_nulls_v11.json +++ b/tests/fixtures/ir_vectors/query_card_nulls_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_card_nulls_v11", + "vector_name": "query_card_nulls_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Card", "where": [ @@ -30,7 +30,8 @@ { "column": "updated_at", "direction": "desc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": null, diff --git a/tests/fixtures/ir_vectors/query_owner_nested_exists_v11.json b/tests/fixtures/ir_vectors/query_owner_nested_exists_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_owner_nested_exists_v11.json rename to tests/fixtures/ir_vectors/query_owner_nested_exists_v14.json index e73fb6b..6b168e2 100644 --- a/tests/fixtures/ir_vectors/query_owner_nested_exists_v11.json +++ b/tests/fixtures/ir_vectors/query_owner_nested_exists_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_owner_nested_exists_v11", + "vector_name": "query_owner_nested_exists_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Owner", "where": [ diff --git a/tests/fixtures/ir_vectors/query_owner_not_exists_v11.json b/tests/fixtures/ir_vectors/query_owner_not_exists_v14.json similarity index 91% rename from tests/fixtures/ir_vectors/query_owner_not_exists_v11.json rename to tests/fixtures/ir_vectors/query_owner_not_exists_v14.json index 5675b58..ce6a403 100644 --- a/tests/fixtures/ir_vectors/query_owner_not_exists_v11.json +++ b/tests/fixtures/ir_vectors/query_owner_not_exists_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_owner_not_exists_v11", + "vector_name": "query_owner_not_exists_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Owner", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_aggregate_v11.json b/tests/fixtures/ir_vectors/query_transaction_aggregate_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_transaction_aggregate_v11.json rename to tests/fixtures/ir_vectors/query_transaction_aggregate_v14.json index 6d1f46a..f760269 100644 --- a/tests/fixtures/ir_vectors/query_transaction_aggregate_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_aggregate_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_aggregate_v11", + "vector_name": "query_transaction_aggregate_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ @@ -24,7 +24,8 @@ { "column": "total", "direction": "desc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": 5, diff --git a/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v11.json b/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v14.json similarity index 95% rename from tests/fixtures/ir_vectors/query_transaction_global_aggregate_v11.json rename to tests/fixtures/ir_vectors/query_transaction_global_aggregate_v14.json index a4ea8a1..7167905 100644 --- a/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_global_aggregate_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_global_aggregate_v11", + "vector_name": "query_transaction_global_aggregate_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_include_v11.json b/tests/fixtures/ir_vectors/query_transaction_include_v14.json similarity index 88% rename from tests/fixtures/ir_vectors/query_transaction_include_v11.json rename to tests/fixtures/ir_vectors/query_transaction_include_v14.json index 7738f0a..65b94de 100644 --- a/tests/fixtures/ir_vectors/query_transaction_include_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_include_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_include_v11", + "vector_name": "query_transaction_include_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ @@ -24,7 +24,8 @@ { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": null, diff --git a/tests/fixtures/ir_vectors/query_transaction_left_join_v11.json b/tests/fixtures/ir_vectors/query_transaction_left_join_v14.json similarity index 94% rename from tests/fixtures/ir_vectors/query_transaction_left_join_v11.json rename to tests/fixtures/ir_vectors/query_transaction_left_join_v14.json index 159a6a1..3383211 100644 --- a/tests/fixtures/ir_vectors/query_transaction_left_join_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_left_join_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_left_join_v11", + "vector_name": "query_transaction_left_join_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ diff --git a/tests/fixtures/ir_vectors/query_transaction_record_v11.json b/tests/fixtures/ir_vectors/query_transaction_record_v14.json similarity index 88% rename from tests/fixtures/ir_vectors/query_transaction_record_v11.json rename to tests/fixtures/ir_vectors/query_transaction_record_v14.json index b87ac6b..a861d3e 100644 --- a/tests/fixtures/ir_vectors/query_transaction_record_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_record_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_record_v11", + "vector_name": "query_transaction_record_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ @@ -24,7 +24,8 @@ { "column": "amount", "direction": "desc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": 25, diff --git a/tests/fixtures/ir_vectors/query_transaction_traversal_v11.json b/tests/fixtures/ir_vectors/query_transaction_traversal_v14.json similarity index 92% rename from tests/fixtures/ir_vectors/query_transaction_traversal_v11.json rename to tests/fixtures/ir_vectors/query_transaction_traversal_v14.json index 0278970..79f1959 100644 --- a/tests/fixtures/ir_vectors/query_transaction_traversal_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_traversal_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_traversal_v11", + "vector_name": "query_transaction_traversal_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ @@ -43,12 +43,14 @@ "direction": "asc", "path": [ "account" - ] + ], + "nulls": "last" }, { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": 50, diff --git a/tests/fixtures/ir_vectors/query_transaction_traversed_record_v11.json b/tests/fixtures/ir_vectors/query_transaction_traversed_record_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_transaction_traversed_record_v11.json rename to tests/fixtures/ir_vectors/query_transaction_traversed_record_v14.json index 393af40..1fced59 100644 --- a/tests/fixtures/ir_vectors/query_transaction_traversed_record_v11.json +++ b/tests/fixtures/ir_vectors/query_transaction_traversed_record_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_transaction_traversed_record_v11", + "vector_name": "query_transaction_traversed_record_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "Transaction", "where": [ @@ -24,7 +24,8 @@ { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": null, diff --git a/tests/fixtures/ir_vectors/query_user_add_columns_set_v11.json b/tests/fixtures/ir_vectors/query_user_add_columns_set_v14.json similarity index 91% rename from tests/fixtures/ir_vectors/query_user_add_columns_set_v11.json rename to tests/fixtures/ir_vectors/query_user_add_columns_set_v14.json index 9bfe2ce..e9babae 100644 --- a/tests/fixtures/ir_vectors/query_user_add_columns_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_add_columns_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_add_columns_set_v11", + "vector_name": "query_user_add_columns_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_add_literal_set_v11.json b/tests/fixtures/ir_vectors/query_user_add_literal_set_v14.json similarity index 92% rename from tests/fixtures/ir_vectors/query_user_add_literal_set_v11.json rename to tests/fixtures/ir_vectors/query_user_add_literal_set_v14.json index c1d8c9d..2dffaa4 100644 --- a/tests/fixtures/ir_vectors/query_user_add_literal_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_add_literal_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_add_literal_set_v11", + "vector_name": "query_user_add_literal_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_after_v14.json b/tests/fixtures/ir_vectors/query_user_after_v14.json new file mode 100644 index 0000000..fd85d91 --- /dev/null +++ b/tests/fixtures/ir_vectors/query_user_after_v14.json @@ -0,0 +1,56 @@ +{ + "vector_name": "query_user_after_v14", + "domain": "query", + "expect_valid": true, + "ir": { + "ir_kind": "query", + "ir_version": 14, + "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_before_v14.json b/tests/fixtures/ir_vectors/query_user_before_v14.json new file mode 100644 index 0000000..d2ac2f2 --- /dev/null +++ b/tests/fixtures/ir_vectors/query_user_before_v14.json @@ -0,0 +1,56 @@ +{ + "vector_name": "query_user_before_v14", + "domain": "query", + "expect_valid": true, + "ir": { + "ir_kind": "query", + "ir_version": 14, + "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, + "before": [ + { + "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_v11.json b/tests/fixtures/ir_vectors/query_user_compound_v14.json similarity index 92% rename from tests/fixtures/ir_vectors/query_user_compound_v11.json rename to tests/fixtures/ir_vectors/query_user_compound_v14.json index 69b546e..13dce52 100644 --- a/tests/fixtures/ir_vectors/query_user_compound_v11.json +++ b/tests/fixtures/ir_vectors/query_user_compound_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_compound_v11", + "vector_name": "query_user_compound_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ @@ -55,7 +55,8 @@ { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": 100, diff --git a/tests/fixtures/ir_vectors/query_user_literal_set_v11.json b/tests/fixtures/ir_vectors/query_user_literal_set_v14.json similarity index 94% rename from tests/fixtures/ir_vectors/query_user_literal_set_v11.json rename to tests/fixtures/ir_vectors/query_user_literal_set_v14.json index 4f42295..a47004e 100644 --- a/tests/fixtures/ir_vectors/query_user_literal_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_literal_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_literal_set_v11", + "vector_name": "query_user_literal_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_m2m_exists_v11.json b/tests/fixtures/ir_vectors/query_user_m2m_exists_v14.json similarity index 94% rename from tests/fixtures/ir_vectors/query_user_m2m_exists_v11.json rename to tests/fixtures/ir_vectors/query_user_m2m_exists_v14.json index 5d6081b..7323da1 100644 --- a/tests/fixtures/ir_vectors/query_user_m2m_exists_v11.json +++ b/tests/fixtures/ir_vectors/query_user_m2m_exists_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_m2m_exists_v11", + "vector_name": "query_user_m2m_exists_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_merge_set_v11.json b/tests/fixtures/ir_vectors/query_user_merge_set_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_merge_set_v11.json rename to tests/fixtures/ir_vectors/query_user_merge_set_v14.json index 62970ad..24fea05 100644 --- a/tests/fixtures/ir_vectors/query_user_merge_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_merge_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_merge_set_v11", + "vector_name": "query_user_merge_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_mixed_set_v11.json b/tests/fixtures/ir_vectors/query_user_mixed_set_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_mixed_set_v11.json rename to tests/fixtures/ir_vectors/query_user_mixed_set_v14.json index 3fa5784..20b0cbd 100644 --- a/tests/fixtures/ir_vectors/query_user_mixed_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_mixed_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_mixed_set_v11", + "vector_name": "query_user_mixed_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_not_compound_v11.json b/tests/fixtures/ir_vectors/query_user_not_compound_v14.json similarity index 93% rename from tests/fixtures/ir_vectors/query_user_not_compound_v11.json rename to tests/fixtures/ir_vectors/query_user_not_compound_v14.json index 93175ff..2fbbf80 100644 --- a/tests/fixtures/ir_vectors/query_user_not_compound_v11.json +++ b/tests/fixtures/ir_vectors/query_user_not_compound_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_not_compound_v11", + "vector_name": "query_user_not_compound_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ diff --git a/tests/fixtures/ir_vectors/query_user_not_leaf_v11.json b/tests/fixtures/ir_vectors/query_user_not_leaf_v14.json similarity index 87% rename from tests/fixtures/ir_vectors/query_user_not_leaf_v11.json rename to tests/fixtures/ir_vectors/query_user_not_leaf_v14.json index b232846..c0ff637 100644 --- a/tests/fixtures/ir_vectors/query_user_not_leaf_v11.json +++ b/tests/fixtures/ir_vectors/query_user_not_leaf_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_not_leaf_v11", + "vector_name": "query_user_not_leaf_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "payload": { "model_name": "User", "where": [ @@ -30,7 +30,8 @@ { "column": "id", "direction": "asc", - "path": [] + "path": [], + "nulls": "last" } ], "limit": 100, diff --git a/tests/fixtures/ir_vectors/query_user_now_set_v11.json b/tests/fixtures/ir_vectors/query_user_now_set_v14.json similarity index 90% rename from tests/fixtures/ir_vectors/query_user_now_set_v11.json rename to tests/fixtures/ir_vectors/query_user_now_set_v14.json index 348dfb5..b9a836c 100644 --- a/tests/fixtures/ir_vectors/query_user_now_set_v11.json +++ b/tests/fixtures/ir_vectors/query_user_now_set_v14.json @@ -1,10 +1,10 @@ { - "vector_name": "query_user_now_set_v11", + "vector_name": "query_user_now_set_v14", "domain": "query", "expect_valid": true, "ir": { "ir_kind": "query", - "ir_version": 11, + "ir_version": 14, "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..67002fa --- /dev/null +++ b/tests/test_after_position.py @@ -0,0 +1,296 @@ +"""``after(position)`` on root order keys, including NULL buckets (#393/#394, 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_accepts_nullable_order_key(): + class AfterNullableKey(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str | None = None + + query = ( + AfterNullableKey.select() + .order_by(lambda row: row.label) + .order_by(lambda row: row.id) + .after(("x", 1)) + ) + assert query._after == ("x", 1) + + +def test_after_none_in_non_pk_slot_is_legal(): + class AfterNullableNone(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str | None = None + + query = ( + AfterNullableNone.select() + .order_by(lambda row: row.label) + .order_by(lambda row: row.id) + .after((None, 1)) + ) + assert query._after == (None, 1) + + +def test_after_none_on_not_null_root_column_is_legal(): + # A NOT NULL column can still be a NULL position slot (left_join missing + # relation — traversal e2e is #396). Nullability is not consulted. + query = _ordered(AfterPageItem).after((None, 1)) + assert query._after == (None, 1) + + +def test_position_of_reads_none_in_non_pk_slot(): + class AfterNullablePos(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str | None = None + + row = AfterNullablePos(id=7, label=None) + position = ( + AfterNullablePos.select() + .order_by(lambda row: row.label) + .order_by(lambda row: row.id) + .position_of(row) + ) + assert position == (None, 7) + + +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_pk_slot_raises(): + with pytest.raises(ValueError, match=r"primary-key|primary key"): + _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] + + +# --------------------------------------------------------------------------- +# E2e: NULL-bucket crossing (Pinch shape — pinned DESC last, then PK). +# --------------------------------------------------------------------------- + + +class AfterPinchConvo(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + pinned_at: datetime | None = None + title: str + + +def _pinch(model: type[AfterPinchConvo] = AfterPinchConvo): + return ( + model.select() + .order_by(lambda convo: convo.pinned_at, "desc") + .order_by(lambda convo: convo.id) + ) + + +async def _seed_pinch_convos() -> list[AfterPinchConvo]: + t1 = datetime(2026, 1, 1, tzinfo=UTC) + t2 = datetime(2026, 2, 1, tzinfo=UTC) + t3 = datetime(2026, 3, 1, tzinfo=UTC) + rows = [ + AfterPinchConvo(id=1, pinned_at=t3, title="pinned-latest"), + AfterPinchConvo(id=2, pinned_at=t2, title="pinned-mid"), + AfterPinchConvo(id=3, pinned_at=t1, title="pinned-oldest"), + AfterPinchConvo(id=4, pinned_at=None, title="unpinned-a"), + AfterPinchConvo(id=5, pinned_at=None, title="unpinned-b"), + AfterPinchConvo(id=6, pinned_at=None, title="unpinned-c"), + ] + for row in rows: + await row.save() + return rows + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_after_none_continues_through_null_bucket(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_pinch_convos() + ordered = await _pinch().all() + assert [row.id for row in ordered] == [1, 2, 3, 4, 5, 6] + + last_pinned = ordered[2] + assert last_pinned.pinned_at is not None + into_unpinned = await _pinch().after((last_pinned.pinned_at, last_pinned.id)).all() + assert [row.id for row in into_unpinned] == [4, 5, 6] + + from_null = await _pinch().after((None, 4)).all() + assert [row.id for row in from_null] == [5, 6] + + position = _pinch().position_of(ordered[3]) + assert position == (None, 4) + from_row = await _pinch().after(ordered[3]).all() + assert [row.id for row in from_row] == [5, 6] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_after_non_null_includes_later_null_bucket(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_pinch_convos() + mid_pinned = await _pinch().after((datetime(2026, 3, 1, tzinfo=UTC), 1)).all() + assert [row.id for row in mid_pinned] == [2, 3, 4, 5, 6] diff --git a/tests/test_before_position.py b/tests/test_before_position.py new file mode 100644 index 0000000..83c5bd2 --- /dev/null +++ b/tests/test_before_position.py @@ -0,0 +1,311 @@ +"""``before(position)`` previous-page paging on root order keys (#395, 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 BeforePageItem(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + updated_at: datetime + name: str + + +def _ordered(item: BeforePageItem): + return ( + BeforePageItem.select() + .order_by(lambda row: row.updated_at) + .order_by(lambda row: row.id) + ) + + +# --------------------------------------------------------------------------- +# Build-time (no DB). +# --------------------------------------------------------------------------- + + +def test_before_without_pk_in_order_keys_raises(): + with pytest.raises(ValueError, match=r"primary key"): + BeforePageItem.select().order_by(lambda row: row.updated_at).before( + (datetime(2026, 1, 1, tzinfo=UTC),) + ) + + +def test_before_on_pkless_model_raises(): + class BeforePkLess(Model): + name: str + rank: int = 0 + + with pytest.raises(ValueError, match=r"BeforePkLess.*no primary-key"): + BeforePkLess.select().order_by(lambda row: row.rank).order_by( + lambda row: row.name + ).before((1, "x")) + + +def test_before_none_in_non_pk_slot_is_legal(): + class BeforeNullableNone(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str | None = None + + query = ( + BeforeNullableNone.select() + .order_by(lambda row: row.label) + .order_by(lambda row: row.id) + .before((None, 1)) + ) + assert query._before == (None, 1) + + +def test_before_none_on_not_null_root_column_is_legal(): + query = _ordered(BeforePageItem).before((None, 1)) + assert query._before == (None, 1) + + +def test_before_wrong_arity_raises(): + with pytest.raises(ValueError, match=r"arity|values"): + _ordered(BeforePageItem).before((datetime(2026, 1, 1, tzinfo=UTC),)) + + +def test_before_none_in_pk_slot_raises(): + with pytest.raises(ValueError, match=r"primary-key|primary key"): + _ordered(BeforePageItem).before((datetime(2026, 1, 1, tzinfo=UTC), None)) + + +def test_before_plus_offset_raises(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + with pytest.raises(ValueError, match=r"offset"): + _ordered(BeforePageItem).offset(1).before((ts, 1)) + with pytest.raises(ValueError, match=r"offset"): + _ordered(BeforePageItem).before((ts, 1)).offset(1) + + +def test_after_plus_before_raises(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + with pytest.raises(ValueError, match=r"after|before"): + _ordered(BeforePageItem).after((ts, 1)).before((ts, 2)) + with pytest.raises(ValueError, match=r"after|before"): + _ordered(BeforePageItem).before((ts, 2)).after((ts, 1)) + + +def test_before_is_immutable(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + base = _ordered(BeforePageItem) + paged = base.before((ts, 1)).limit(2) + assert paged is not base + assert base._before is None + assert paged._before == (ts, 1) + assert paged._limit == 2 + assert base._limit is None + + +def test_update_and_delete_reject_before(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + query = _ordered(BeforePageItem).before((ts, 1)) + with pytest.raises(ValueError, match=r"before"): + compile_query(query, "update", assignments={"name": "x"}) + with pytest.raises(ValueError, match=r"before"): + compile_query(query, "delete") + + +def test_count_drops_before_from_the_wire(): + ts = datetime(2026, 1, 1, tzinfo=UTC) + query = _ordered(BeforePageItem).before((ts, 1)).limit(3) + payload = compile_query(query, "count").payload.to_ir_dict() + assert "before" not in payload + assert payload["limit"] is None + assert payload["offset"] is None + + +def test_fetch_omits_before_when_unset(): + payload = compile_query(_ordered(BeforePageItem), "fetch").payload.to_ir_dict() + assert "before" not in payload + assert "after" not in payload + + +# --------------------------------------------------------------------------- +# E2e: adjacent previous page in declared order. +# --------------------------------------------------------------------------- + + +async def _seed_page_items() -> list[BeforePageItem]: + rows = [ + BeforePageItem(id=1, updated_at=datetime(2026, 1, 1, tzinfo=UTC), name="a"), + BeforePageItem(id=2, updated_at=datetime(2026, 1, 1, tzinfo=UTC), name="b"), + BeforePageItem(id=3, updated_at=datetime(2026, 2, 1, tzinfo=UTC), name="c"), + BeforePageItem(id=4, updated_at=datetime(2026, 3, 1, tzinfo=UTC), name="d"), + BeforePageItem(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_before_limit_returns_adjacent_previous_page_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(BeforePageItem).all() + assert [row.id for row in ordered] == [1, 2, 3, 4, 5] + + page = ( + await _ordered(BeforePageItem) + .before((ordered[3].updated_at, ordered[3].id)) + .limit(2) + .all() + ) + assert [row.id for row in page] == [2, 3] + assert ordered[3] not in page + + past_start = ( + await _ordered(BeforePageItem) + .before((ordered[0].updated_at, ordered[0].id)) + .limit(2) + .all() + ) + assert past_start == [] + + assert ( + await _ordered(BeforePageItem) + .before((ordered[3].updated_at, ordered[3].id)) + .count() + == 5 + ) + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_unbounded_before_returns_every_earlier_row_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(BeforePageItem).all() + prefix = await _ordered(BeforePageItem).before( + (ordered[3].updated_at, ordered[3].id) + ).all() + assert [row.id for row in prefix] == [1, 2, 3] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_before_first_is_adjacent_previous_row(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_page_items() + ordered = await _ordered(BeforePageItem).all() + adjacent = await _ordered(BeforePageItem).before( + (ordered[3].updated_at, ordered[3].id) + ).first() + assert adjacent is not None + assert adjacent.id == 3 + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_unbounded_before_first_disagrees_with_all_head(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_page_items() + ordered = await _ordered(BeforePageItem).all() + query = _ordered(BeforePageItem).before((ordered[3].updated_at, ordered[3].id)) + prefix = await query.all() + adjacent = await query.first() + assert [row.id for row in prefix] == [1, 2, 3] + assert prefix[0].id == 1 + assert adjacent is not None + assert adjacent.id == 3 + assert adjacent.id != prefix[0].id + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_before_row_equals_before_tuple(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_page_items() + query = _ordered(BeforePageItem) + rows = await query.all() + anchor = rows[3] + position = query.position_of(anchor) + assert position == (anchor.updated_at, anchor.id) + + from_tuple = await query.before(position).limit(2).all() + from_row = await query.before(anchor).limit(2).all() + assert [row.id for row in from_tuple] == [2, 3] + assert [row.id for row in from_row] == [2, 3] + + +# --------------------------------------------------------------------------- +# E2e: NULL-bucket crossing (Pinch shape — pinned DESC last, then PK). +# --------------------------------------------------------------------------- + + +class BeforePinchConvo(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + pinned_at: datetime | None = None + title: str + + +def _pinch(model: type[BeforePinchConvo] = BeforePinchConvo): + return ( + model.select() + .order_by(lambda convo: convo.pinned_at, "desc") + .order_by(lambda convo: convo.id) + ) + + +async def _seed_pinch_convos() -> list[BeforePinchConvo]: + t1 = datetime(2026, 1, 1, tzinfo=UTC) + t2 = datetime(2026, 2, 1, tzinfo=UTC) + t3 = datetime(2026, 3, 1, tzinfo=UTC) + rows = [ + BeforePinchConvo(id=1, pinned_at=t3, title="pinned-latest"), + BeforePinchConvo(id=2, pinned_at=t2, title="pinned-mid"), + BeforePinchConvo(id=3, pinned_at=t1, title="pinned-oldest"), + BeforePinchConvo(id=4, pinned_at=None, title="unpinned-a"), + BeforePinchConvo(id=5, pinned_at=None, title="unpinned-b"), + BeforePinchConvo(id=6, pinned_at=None, title="unpinned-c"), + ] + for row in rows: + await row.save() + return rows + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_before_crosses_null_bucket_into_pinned(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_pinch_convos() + ordered = await _pinch().all() + assert [row.id for row in ordered] == [1, 2, 3, 4, 5, 6] + + first_unpinned = ordered[3] + assert first_unpinned.pinned_at is None + adjacent = await _pinch().before((None, first_unpinned.id)).limit(1).all() + assert [row.id for row in adjacent] == [3] + + page = await _pinch().before((None, first_unpinned.id)).limit(2).all() + assert [row.id for row in page] == [2, 3] + + prefix = await _pinch().before((None, first_unpinned.id)).all() + assert [row.id for row in prefix] == [1, 2, 3] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_before_null_bound_stays_in_null_bucket_when_later(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_pinch_convos() + earlier_unpinned = await _pinch().before((None, 6)).limit(2).all() + assert [row.id for row in earlier_unpinned] == [4, 5] diff --git a/tests/test_ir_vectors_contract.py b/tests/test_ir_vectors_contract.py index 0142f4b..a6f7378 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 11 (#379 — unconditional bump; merge joins -# add / sub / now); `schema`/`codec` remain v1. -SUPPORTED_IR_VERSIONS = {"schema": 1, "query": 11, "codec": 1} +# `query` is on ir_version 14 (#395 — optional `before` position bound); +# `schema`/`codec` remain v1. +SUPPORTED_IR_VERSIONS = {"schema": 1, "query": 14, "codec": 1} QUERY_OPERATORS = {"==", "!=", "<", "<=", ">", ">=", "IN", "LIKE", "AND", "OR"} MATERIALIZATION_KINDS = {"root_instances", "record", "instances"} AGGREGATE_FNS = {"count", "sum", "avg", "min", "max"} @@ -285,13 +285,12 @@ def _validate_query_payload(payload: dict[str, Any], label: str) -> None: for i, order in enumerate(payload["order_by"]): order_label = f"{label}.order_by[{i}]" assert isinstance(order, dict), f"{order_label} must be object" - _require_keys(order, {"column", "direction", "path"}, order_label) + _require_keys(order, {"column", "direction", "path", "nulls"}, order_label) assert isinstance(order["path"], list), f"{order_label}.path must be a list" - if "nulls" in order: - assert order["nulls"] in {"first", "last"}, ( - f"{order_label}.nulls must be 'first' or 'last' when present, " - f"got {order['nulls']!r}" - ) + assert order["nulls"] in {"first", "last", "native"}, ( + f"{order_label}.nulls must be 'first', 'last', or 'native', " + f"got {order['nulls']!r}" + ) has_limit = "limit" in payload has_offset = "offset" in payload assert has_limit == has_offset, f"{label}.limit/offset must be present together" @@ -303,6 +302,33 @@ 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 "before" in payload: + before = payload["before"] + assert isinstance(before, list) and before, f"{label}.before must be a non-empty list" + assert len(before) == len(payload["order_by"]), ( + f"{label}.before arity must match order_by" + ) + for i, value in enumerate(before): + value_label = f"{label}.before[{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 before and a non-null offset" + ) + assert "after" not in payload, f"{label} cannot carry after and before" 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..760aaf4 100644 --- a/tests/test_mutation_pagination_guard.py +++ b/tests/test_mutation_pagination_guard.py @@ -59,6 +59,8 @@ 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 "before" 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.py b/tests/test_order_by_nulls.py index 9c8bdb3..a7768c2 100644 --- a/tests/test_order_by_nulls.py +++ b/tests/test_order_by_nulls.py @@ -1,8 +1,8 @@ -"""Backend-matrix e2e for ``order_by(..., nulls=...)`` placement (#363). +"""Backend-matrix e2e for ``order_by(..., nulls=...)`` placement (#363, #392). Asserts result-set order only — same row order on SQLite and Postgres when -``nulls=`` is set. Omitted-``nulls`` DESC on a nullable column is deliberately -not cross-backend-asserted (dialect defaults diverge). +``nulls=`` is set. Omitted ``nulls=`` compiles to ``last`` and is +cross-backend-asserted. """ from typing import Annotated @@ -94,6 +94,23 @@ async def _seed_items() -> None: # --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_omitted_nulls_means_last_on_both_backends(db_url): + """Omitted nulls= on DESC sorts set values first, NULLs last, on both backends.""" + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_cards() + + rows = await ( + ObnCard.select() + .order_by(lambda c: c.pinned_at, "desc") + .order_by(lambda c: c.id) + .all() + ) + + assert [r.id for r in rows] == [3, 1, 5, 2, 4] + + @pytest.mark.asyncio async def test_desc_nulls_last_same_order_on_both_backends(db_url): """DESC + nulls=\"last\": set values lead, NULLs trail; id tiebreaker.""" diff --git a/tests/test_order_by_nulls_wire.py b/tests/test_order_by_nulls_wire.py index ab92ed6..6e3d5c7 100644 --- a/tests/test_order_by_nulls_wire.py +++ b/tests/test_order_by_nulls_wire.py @@ -1,4 +1,4 @@ -"""Build-time wire shape for ``order_by(..., nulls=...)`` (#362). +"""Build-time wire shape for ``order_by(..., nulls=...)`` (#362, #392). Stops at ``compile_query`` — no SQL execution, no row-order assertions. """ @@ -14,7 +14,7 @@ pytestmark = pytest.mark.sqlite_only -def test_compile_query_emits_nulls_only_when_set(): +def test_compile_query_emits_nulls_on_every_term(): class WireCard(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None pinned_at: str | None = None @@ -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"] == 11 + assert envelope["ir_version"] == 14 order_by = envelope["payload"]["order_by"] assert order_by == [ { @@ -39,9 +39,9 @@ class WireCard(Model): "column": "updated_at", "direction": "desc", "path": [], + "nulls": "last", }, ] - assert "nulls" not in order_by[1] def test_compile_query_nulls_first_on_projected_query(): @@ -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"] == 11 + assert envelope["ir_version"] == 14 assert envelope["payload"]["order_by"] == [ { "column": "total", @@ -88,7 +88,28 @@ class WirePost(Model): ] -def test_compile_query_omitted_nulls_keeps_ir_version_11(): +def test_compile_query_native_nulls_on_wire(): + class WireNative(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + age: int = 0 + + envelope = json.loads( + compile_query( + WireNative.select().order_by("age", "desc", nulls="native"), + "fetch", + ).wire_json + ) + assert envelope["ir_version"] == 14 + entry = envelope["payload"]["order_by"][0] + assert entry == { + "column": "age", + "direction": "desc", + "path": [], + "nulls": "native", + } + + +def test_compile_query_omitted_nulls_emits_last_at_ir_version_14(): class WirePlain(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None age: int = 0 @@ -96,7 +117,11 @@ class WirePlain(Model): envelope = json.loads( compile_query(WirePlain.select().order_by("age"), "fetch").wire_json ) - assert envelope["ir_version"] == 11 + assert envelope["ir_version"] == 14 entry = envelope["payload"]["order_by"][0] - assert entry == {"column": "age", "direction": "asc", "path": []} - assert "nulls" not in entry + assert entry == { + "column": "age", + "direction": "asc", + "path": [], + "nulls": "last", + } diff --git a/tests/test_position_paging_traversal.py b/tests/test_position_paging_traversal.py new file mode 100644 index 0000000..6264a38 --- /dev/null +++ b/tests/test_position_paging_traversal.py @@ -0,0 +1,291 @@ +"""Position paging on traversed order keys and projected records (#396, ADR-0018). + +Build-time tests need no database. E2e tests run on the backend matrix. +A decoded tuple is enough to page — ``include()`` is not required. +""" + +from typing import Annotated + +import pytest + +import ferro +from ferro import BackRef, FerroField, ForeignKey, Model, Relation +from ferro.query.rows import Row + + +class PosTravAccount(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str + transactions: Relation[list["PosTravTransaction"]] = BackRef() + notes: Relation[list["PosTravNote"]] = BackRef() + + +class PosTravTransaction(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + amount: int = 0 + account: Annotated[PosTravAccount, ForeignKey(related_name="transactions")] + + +class PosTravNote(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + body: str = "" + account: Annotated[PosTravAccount | None, ForeignKey(related_name="notes")] = None + + +class PosTravAggItem(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + category: str + amount: int = 0 + + +def _traversed(model: type[PosTravTransaction] = PosTravTransaction): + return ( + model.select() + .order_by(lambda txn: txn.account.label) + .order_by(lambda txn: txn.id) + ) + + +# --------------------------------------------------------------------------- +# Build-time (no DB). +# --------------------------------------------------------------------------- + + +def test_after_accepts_traversed_order_key_from_a_tuple(): + query = _traversed().after(("a1", 1)) + assert query._after == ("a1", 1) + + +def test_before_accepts_traversed_order_key_from_a_tuple(): + query = _traversed().before(("a1", 1)) + assert query._before == ("a1", 1) + + +def test_related_id_is_not_the_model_primary_key(): + with pytest.raises(ValueError, match=r"primary key"): + (PosTravTransaction.select().order_by(lambda txn: txn.account.id).after((1,))) + + +def test_position_of_unpopulated_traversed_key_raises(): + row = PosTravTransaction.model_construct(id=1, amount=10, account_id=1) + with pytest.raises(ValueError, match=r"account\.label"): + _traversed().position_of(row) + + +def test_position_of_populated_traversed_key_reads_the_related_column(): + account = PosTravAccount.model_construct(id=7, label="a1") + row = PosTravTransaction.model_construct(id=1, amount=10, account_id=7) + # include() population: relation name in __dict__ shadows ForwardDescriptor. + row.__dict__["account"] = account + assert _traversed().position_of(row) == ("a1", 1) + + +def test_position_of_populated_none_related_slot_is_legal(): + row = PosTravNote.model_construct(id=3, body="orphan") + row.__dict__["account"] = None + position = ( + PosTravNote.select() + .order_by(lambda note: note.account.label) + .order_by(lambda note: note.id) + .position_of(row) + ) + assert position == (None, 3) + + +def test_after_none_in_left_joined_related_slot_is_legal(): + query = ( + PosTravNote.select() + .left_join(lambda note: note.account) + .order_by(lambda note: note.account.label) + .order_by(lambda note: note.id) + .after((None, 3)) + ) + assert query._after == (None, 3) + + +def test_projected_after_accepts_a_tuple(): + query = ( + PosTravTransaction.select( + lambda txn: {"label": txn.account.label, "id": txn.id} + ) + .order_by(lambda txn: txn.account.label) + .order_by(lambda txn: txn.id) + .after(("a1", 1)) + ) + assert query._after == ("a1", 1) + + +def test_projected_position_of_missing_order_key_raises(): + query = ( + PosTravTransaction.select(lambda txn: (txn.id, txn.amount)) + .order_by(lambda txn: txn.account.label) + .order_by(lambda txn: txn.id) + ) + row = Row.model_construct(id=1, amount=10) + with pytest.raises(ValueError, match=r"tuple"): + query.position_of(row) + + +def test_projected_position_of_reads_selected_order_keys(): + query = ( + PosTravTransaction.select( + lambda txn: {"label": txn.account.label, "id": txn.id} + ) + .order_by(lambda txn: txn.account.label) + .order_by(lambda txn: txn.id) + ) + row = Row.model_construct(label="a1", id=1) + assert query.position_of(row) == ("a1", 1) + + +def test_grouped_aggregate_after_raises_for_missing_pk(): + query = ( + PosTravAggItem.select( + lambda item: {"cat": item.category, "total": item.amount.sum()} + ) + .order_by("total", "desc") + .order_by("cat") + ) + with pytest.raises(ValueError, match=r"primary key"): + query.after((100, "food")) + + +def test_grouped_aggregate_before_raises_for_missing_pk(): + query = ( + PosTravAggItem.select( + lambda item: {"cat": item.category, "total": item.amount.sum()} + ) + .order_by("total", "desc") + .order_by("cat") + ) + with pytest.raises(ValueError, match=r"primary key"): + query.before((100, "food")) + + +def test_aggregate_order_key_raises_even_when_pk_is_a_group_key(): + query = ( + PosTravAggItem.select(lambda item: {"id": item.id, "total": item.amount.sum()}) + .order_by("total") + .order_by("id") + ) + with pytest.raises(ValueError, match=r"aggregate"): + query.after((100, 1)) + + +def test_expression_order_key_remains_a_build_time_error(): + with pytest.raises(TypeError, match=r"value expressions|FieldProxy"): + PosTravTransaction.select().order_by(lambda txn: txn.amount + 1) + + +# --------------------------------------------------------------------------- +# E2e: traversed after/before from a tuple, no include(). +# --------------------------------------------------------------------------- + + +async def _seed_transactions() -> None: + a1 = await PosTravAccount.create(id=1, label="a1") + a2 = await PosTravAccount.create(id=2, label="b1") + await PosTravTransaction.create(id=1, amount=10, account=a1) + await PosTravTransaction.create(id=2, amount=20, account=a1) + await PosTravTransaction.create(id=3, amount=30, account=a2) + await PosTravTransaction.create(id=4, amount=40, account=a2) + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_after_traversed_tuple_pages_without_include(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_transactions() + ordered = await _traversed().all() + assert [row.id for row in ordered] == [1, 2, 3, 4] + + page = await _traversed().after(("a1", 2)).limit(2).all() + assert [row.id for row in page] == [3, 4] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_before_traversed_tuple_pages_without_include(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_transactions() + page = await _traversed().before(("b1", 3)).limit(2).all() + assert [row.id for row in page] == [1, 2] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_position_of_instance_requires_populated_path(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_transactions() + bare = await PosTravTransaction.select().where(lambda txn: txn.id == 2).first() + assert bare is not None + with pytest.raises(ValueError, match=r"account\.label"): + _traversed().position_of(bare) + + populated = await ( + PosTravTransaction.select() + .include(lambda txn: txn.account) + .where(lambda txn: txn.id == 2) + .first() + ) + assert populated is not None + assert _traversed().position_of(populated) == ("a1", 2) + page = await _traversed().after(populated).limit(2).all() + assert [row.id for row in page] == [3, 4] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_left_join_none_related_slot_pages(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + a1 = await PosTravAccount.create(id=1, label="a1") + await PosTravNote.create(id=1, body="on-a1", account=a1) + await PosTravNote.create(id=2, body="orphan") + + query = ( + PosTravNote.select() + .left_join(lambda note: note.account) + .order_by(lambda note: note.account.label) + .order_by(lambda note: note.id) + ) + ordered = await query.all() + assert [row.id for row in ordered] == [1, 2] + + after_related = await query.after(("a1", 1)).all() + assert [row.id for row in after_related] == [2] + + after_null = await query.after((None, 2)).all() + assert after_null == [] + + +@pytest.mark.backend_matrix +@pytest.mark.asyncio +async def test_projected_record_after_before_and_position_of(db_url): + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_transactions() + query = ( + PosTravTransaction.select( + lambda txn: {"label": txn.account.label, "id": txn.id} + ) + .order_by(lambda txn: txn.account.label) + .order_by(lambda txn: txn.id) + ) + rows = await query.all() + assert [row.id for row in rows] == [1, 2, 3, 4] + + position = query.position_of(rows[1]) + assert position == ("a1", 2) + + after_page = await query.after(position).limit(2).all() + assert [row.id for row in after_page] == [3, 4] + + from_row = await query.after(rows[1]).limit(2).all() + assert [row.id for row in from_row] == [3, 4] + + before_page = await query.before(("b1", 3)).limit(2).all() + assert [row.id for row in before_page] == [1, 2] diff --git a/tests/test_query_builder.py b/tests/test_query_builder.py index 92f0328..aa19eed 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"] == 11 + assert payload["ir_version"] == 14 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_column_validation.py b/tests/test_query_column_validation.py index b208859..3f754ed 100644 --- a/tests/test_query_column_validation.py +++ b/tests/test_query_column_validation.py @@ -165,9 +165,9 @@ class ObNullsJunk(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None pinned_at: str | None = None - with pytest.raises(ValueError, match=r"first.*last"): + with pytest.raises(ValueError, match=r"first.*last.*native"): ObNullsJunk.select().order_by("pinned_at", nulls="sideways") - with pytest.raises(ValueError, match=r"first.*last"): + with pytest.raises(ValueError, match=r"first.*last.*native"): ObNullsJunk.select().order_by( lambda u: u.pinned_at, "desc", nulls="nulls last" ) @@ -191,16 +191,28 @@ class ObNotNull(Model): OrderByEntry(column="name", direction="asc", path=(), nulls="last") ] - def test_order_by_omitted_nulls_matches_legacy_entry(self): + def test_order_by_nulls_native_accepted(self): + class ObNative(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + pinned_at: str | None = None + + q = ObNative.select().order_by("pinned_at", "desc", nulls="native") + assert q.order_by_clause == [ + OrderByEntry( + column="pinned_at", direction="desc", path=(), nulls="native" + ) + ] + + def test_order_by_omitted_nulls_defaults_to_last(self): class ObOmit(Model): id: Annotated[int | None, FerroField(primary_key=True)] = None age: int = 0 q = ObOmit.select().order_by(lambda u: u.age, "desc") assert q.order_by_clause == [ - OrderByEntry(column="age", direction="desc", path=()) + OrderByEntry(column="age", direction="desc", path=(), nulls="last") ] - assert q.order_by_clause[0].nulls is None + assert q.order_by_clause[0].nulls == "last" def test_order_by_nulls_with_relation_traversal(self): class ObAuthor(Model): diff --git a/tests/test_query_immutability.py b/tests/test_query_immutability.py index b460b36..ae3d720 100644 --- a/tests/test_query_immutability.py +++ b/tests/test_query_immutability.py @@ -54,6 +54,28 @@ 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_before_does_not_mutate(self): + class ImmUserBefore(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + age: int = 0 + + q1 = Query(ImmUserBefore).order_by("age").order_by("id") + q2 = q1.before((18, 1)).limit(2) + assert q1._before is None + assert q2._before == (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_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 diff --git a/tests/test_query_wire_vectors.py b/tests/test_query_wire_vectors.py index 5466e63..965b5f8 100644 --- a/tests/test_query_wire_vectors.py +++ b/tests/test_query_wire_vectors.py @@ -280,7 +280,7 @@ def _q_m2m_exists(m: dict[str, type]) -> Any: def _q_card_nulls(m: dict[str, type]) -> Any: - # #363: first order_by term carries nulls=, the next omits it. + # #392: first order_by term carries explicit nulls=; omitted compiles to last. return ( m["Card"] .select() @@ -290,23 +290,61 @@ 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) + ) + + +def _q_user_before(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) + .before((10, 3)) + .limit(5) + ) + + +def _q_card_after_null_slot(m: dict[str, type]) -> Any: + return ( + m["Card"] + .select() + .where(lambda c: c.id != None) # noqa: E711 + .order_by(lambda c: c.pinned_at, "desc") + .order_by(lambda c: c.id) + .after((None, 4)) + ) + + CASES: list[tuple[str, Callable[[dict[str, type]], Any], str]] = [ - ("query_user_compound_v11", _q_user_compound, "User"), - ("query_user_not_leaf_v11", _q_not_leaf, "User"), - ("query_user_not_compound_v11", _q_not_compound, "User"), - ("query_account_exists_v11", _q_exists_bare, "Account"), - ("query_owner_not_exists_v11", _q_not_exists, "Owner"), - ("query_account_scoped_exists_v11", _q_scoped_exists, "Account"), - ("query_owner_nested_exists_v11", _q_nested_exists, "Owner"), - ("query_user_m2m_exists_v11", _q_m2m_exists, "User"), - ("query_transaction_traversal_v11", _q_traversal, "Transaction"), - ("query_transaction_left_join_v11", _q_left_join, "Transaction"), - ("query_transaction_include_v11", _q_include, "Transaction"), - ("query_transaction_record_v11", _q_record, "Transaction"), - ("query_transaction_traversed_record_v11", _q_traversed_record, "Transaction"), - ("query_transaction_aggregate_v11", _q_aggregate, "Transaction"), - ("query_transaction_global_aggregate_v11", _q_global_aggregate, "Transaction"), - ("query_card_nulls_v11", _q_card_nulls, "Card"), + ("query_user_compound_v14", _q_user_compound, "User"), + ("query_user_not_leaf_v14", _q_not_leaf, "User"), + ("query_user_not_compound_v14", _q_not_compound, "User"), + ("query_account_exists_v14", _q_exists_bare, "Account"), + ("query_owner_not_exists_v14", _q_not_exists, "Owner"), + ("query_account_scoped_exists_v14", _q_scoped_exists, "Account"), + ("query_owner_nested_exists_v14", _q_nested_exists, "Owner"), + ("query_user_m2m_exists_v14", _q_m2m_exists, "User"), + ("query_transaction_traversal_v14", _q_traversal, "Transaction"), + ("query_transaction_left_join_v14", _q_left_join, "Transaction"), + ("query_transaction_include_v14", _q_include, "Transaction"), + ("query_transaction_record_v14", _q_record, "Transaction"), + ("query_transaction_traversed_record_v14", _q_traversed_record, "Transaction"), + ("query_transaction_aggregate_v14", _q_aggregate, "Transaction"), + ("query_transaction_global_aggregate_v14", _q_global_aggregate, "Transaction"), + ("query_card_nulls_v14", _q_card_nulls, "Card"), + ("query_user_after_v14", _q_user_after, "User"), + ("query_user_before_v14", _q_user_before, "User"), + ("query_card_after_null_slot_v14", _q_card_after_null_slot, "Card"), ] @@ -352,10 +390,27 @@ 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_drops_before_bound(models: dict[str, type]) -> None: + query = models["User"].select().order_by("id").before((1,)).limit(3) + payload = _payload(query, "count") + assert "before" 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 +433,8 @@ 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 "before" not in payload assert payload["order_by"] == [] assert payload["m2m"] is None assert payload["joins"] == [] @@ -387,7 +444,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_v11") + vector = _vector("query_user_literal_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -408,7 +465,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_v11") + vector = _vector("query_user_mixed_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -486,13 +543,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"] == 11 + assert envelope["ir_version"] == 14 def test_binary_add_column_literal_matches_hand_authored_vector( models: dict[str, type], ) -> None: - vector = _vector("query_user_add_literal_set_v11") + vector = _vector("query_user_add_literal_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -512,7 +569,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_v11") + vector = _vector("query_user_add_columns_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -531,7 +588,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_v11") + vector = _vector("query_user_now_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User" @@ -549,7 +606,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_v11") + vector = _vector("query_user_merge_set_v14") expected = vector["ir"] assert expected["payload"]["model_name"] == "User"