diff --git a/crates/ferro-schema-ir/src/lib.rs b/crates/ferro-schema-ir/src/lib.rs index 24d1322..33e4432 100644 --- a/crates/ferro-schema-ir/src/lib.rs +++ b/crates/ferro-schema-ir/src/lib.rs @@ -1347,6 +1347,35 @@ mod tests { ); } + #[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_v7.json"); + let parsed: serde_json::Value = + serde_json::from_str(fixture).expect("query card-nulls 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 card-nulls IR must deserialize"); + assert_eq!(envelope.ir_version, 7); + 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" + ); + 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 codec_fixture_roundtrip() { let fixture = diff --git a/tests/fixtures/ir_vectors/query_card_nulls_v7.json b/tests/fixtures/ir_vectors/query_card_nulls_v7.json new file mode 100644 index 0000000..f73c354 --- /dev/null +++ b/tests/fixtures/ir_vectors/query_card_nulls_v7.json @@ -0,0 +1,44 @@ +{ + "vector_name": "query_card_nulls_v7", + "domain": "query", + "expect_valid": true, + "ir": { + "ir_kind": "query", + "ir_version": 7, + "payload": { + "model_name": "Card", + "where": [ + { + "node_kind": "leaf", + "column": "id", + "operator": "!=", + "value": { + "kind": "null", + "value": null + }, + "path": [] + } + ], + "order_by": [ + { + "column": "pinned_at", + "direction": "desc", + "path": [], + "nulls": "last" + }, + { + "column": "updated_at", + "direction": "desc", + "path": [] + } + ], + "limit": null, + "offset": null, + "m2m": null, + "joins": [], + "materialization": { + "kind": "root_instances" + } + } + } +} diff --git a/tests/test_ir_vectors_contract.py b/tests/test_ir_vectors_contract.py index 04f4c4a..31f5fbf 100644 --- a/tests/test_ir_vectors_contract.py +++ b/tests/test_ir_vectors_contract.py @@ -253,6 +253,11 @@ def _validate_query_payload(payload: dict[str, Any], label: str) -> None: assert isinstance(order, dict), f"{order_label} must be object" _require_keys(order, {"column", "direction", "path"}, 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}" + ) if payload["limit"] is not None: assert isinstance(payload["limit"], int) and payload["limit"] >= 0, ( f"{label}.limit must be null or non-negative int" diff --git a/tests/test_order_by_nulls.py b/tests/test_order_by_nulls.py new file mode 100644 index 0000000..9c8bdb3 --- /dev/null +++ b/tests/test_order_by_nulls.py @@ -0,0 +1,209 @@ +"""Backend-matrix e2e for ``order_by(..., nulls=...)`` placement (#363). + +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). +""" + +from typing import Annotated + +import pytest + +import ferro +from ferro import BackRef, FerroField, ForeignKey, Model, Relation + +pytestmark = pytest.mark.backend_matrix + + +# --------------------------------------------------------------------------- +# Root nullable sort key + unique id tiebreaker. +# --------------------------------------------------------------------------- + + +class ObnCard(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + pinned_at: str | None = None + name: str = "" + + +async def _seed_cards() -> None: + """Mixed set + NULL pinned_at values; unique ids for the tiebreaker.""" + for row in ( + ObnCard(id=1, pinned_at="2024-06-01", name="mid"), + ObnCard(id=2, pinned_at=None, name="null-a"), + ObnCard(id=3, pinned_at="2024-12-01", name="late"), + ObnCard(id=4, pinned_at=None, name="null-b"), + ObnCard(id=5, pinned_at="2024-01-01", name="early"), + ): + await row.save() + + +# --------------------------------------------------------------------------- +# Left-join traversal: related column is NOT NULL on the related model; +# NULLs appear because the relation is missing. +# --------------------------------------------------------------------------- + + +class ObnBoard(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + title: str = "" + cards: Relation[list["ObnBoardCard"]] = BackRef() + + +class ObnBoardCard(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + label: str = "" + board: Annotated[ObnBoard | None, ForeignKey(related_name="cards")] = None + + +async def _seed_board_cards() -> None: + zeta = ObnBoard(id=1, title="zeta") + alpha = ObnBoard(id=2, title="alpha") + await zeta.save() + await alpha.save() + await ObnBoardCard(id=1, label="on-zeta", board=zeta).save() + await ObnBoardCard(id=2, label="on-alpha", board=alpha).save() + await ObnBoardCard(id=3, label="orphan", board=None).save() + + +# --------------------------------------------------------------------------- +# Grouped aggregate: SUM of all-NULL amounts is NULL (empty numeric input). +# --------------------------------------------------------------------------- + + +class ObnItem(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + category: str = "" + amount: int | None = None + + +async def _seed_items() -> None: + for row in ( + ObnItem(id=1, category="a", amount=100), + ObnItem(id=2, category="a", amount=50), + ObnItem(id=3, category="b", amount=None), + ObnItem(id=4, category="c", amount=10), + ObnItem(id=5, category="d", amount=None), + ObnItem(id=6, category="d", amount=None), + ): + await row.save() + + +# --------------------------------------------------------------------------- +# Acceptance. +# --------------------------------------------------------------------------- + + +@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.""" + 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", nulls="last") + .order_by(lambda c: c.id) + .all() + ) + + assert [r.id for r in rows] == [3, 1, 5, 2, 4] + assert [r.pinned_at for r in rows] == [ + "2024-12-01", + "2024-06-01", + "2024-01-01", + None, + None, + ] + + +@pytest.mark.asyncio +async def test_desc_nulls_first_same_order_on_both_backends(db_url): + """nulls=\"first\" on the same seed: NULLs lead; id tiebreaker among NULLs.""" + 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", nulls="first") + .order_by(lambda c: c.id) + .all() + ) + + assert [r.id for r in rows] == [2, 4, 3, 1, 5] + + +@pytest.mark.asyncio +async def test_chained_terms_first_carries_nulls_later_do_not(db_url): + """First term carries nulls=; later tiebreakers omit it — order matches SQL.""" + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + # Two rows share pinned_at=None; tiebreak on name DESC then id ASC. + for row in ( + ObnCard(id=1, pinned_at="2024-06-01", name="mid"), + ObnCard(id=2, pinned_at=None, name="zebra"), + ObnCard(id=3, pinned_at=None, name="alpha"), + ObnCard(id=4, pinned_at="2024-12-01", name="late"), + ): + await row.save() + + rows = await ( + ObnCard.select() + .order_by(lambda c: c.pinned_at, "desc", nulls="last") + .order_by(lambda c: c.name, "desc") + .order_by(lambda c: c.id) + .all() + ) + + # Set values DESC (12-01, 06-01), then NULLs with name DESC (zebra, alpha). + assert [r.id for r in rows] == [4, 1, 2, 3] + + +@pytest.mark.asyncio +async def test_left_join_related_not_null_column_honors_nulls(db_url): + """Traversed forward-FK: board.title is NOT NULL on Board; LEFT join NULLs + from missing relation honor nulls= placement on both backends.""" + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_board_cards() + + last = await ( + ObnBoardCard.select() + .left_join(lambda c: c.board) + .order_by(lambda c: c.board.title, "asc", nulls="last") + .order_by(lambda c: c.id) + .all() + ) + assert [r.id for r in last] == [2, 1, 3] + + first = await ( + ObnBoardCard.select() + .left_join(lambda c: c.board) + .order_by(lambda c: c.board.title, "asc", nulls="first") + .order_by(lambda c: c.id) + .all() + ) + assert [r.id for r in first] == [3, 2, 1] + + +@pytest.mark.asyncio +async def test_projected_aggregate_sum_nulls_last(db_url): + """order_by(lambda t: t.amount.sum(), \"desc\", nulls=\"last\") — SUM of + all-NULL groups is NULL and trails the set totals on both backends.""" + await ferro.connect(db_url, auto_migrate=True) + async with ferro.engines.session(): + await _seed_items() + + rows = await ( + ObnItem.select( + lambda t: {"category": t.category, "total": t.amount.sum()} + ) + .order_by(lambda t: t.amount.sum(), "desc", nulls="last") + .order_by("category") + .all() + ) + + assert [r.category for r in rows] == ["a", "c", "b", "d"] + assert [r.total for r in rows] == [150, 10, None, None] diff --git a/tests/test_query_wire_vectors.py b/tests/test_query_wire_vectors.py index 0ccfb9a..d71f15c 100644 --- a/tests/test_query_wire_vectors.py +++ b/tests/test_query_wire_vectors.py @@ -76,6 +76,13 @@ class Tag(Model): name: str = "" users: Relation[list["User"]] = ManyToMany(related_name="tags") + # Dedicated model for the nulls= golden vector (#363) — do not hang + # pinned_at / updated_at onto User/Account/Transaction (existing fixtures). + class Card(Model): + id: Annotated[int | None, FerroField(primary_key=True)] = None + pinned_at: str | None = None + updated_at: str = "" + resolve_relationships() return { "Owner": Owner, @@ -83,6 +90,7 @@ class Tag(Model): "Transaction": Transaction, "User": User, "Tag": Tag, + "Card": Card, } @@ -261,6 +269,17 @@ def _q_m2m_exists(m: dict[str, type]) -> Any: return m["User"].where(lambda u: u.tags.exists(lambda tag: tag.name == "admin")) +def _q_card_nulls(m: dict[str, type]) -> Any: + # #363: first order_by term carries nulls=, the next omits it. + return ( + m["Card"] + .select() + .where(lambda c: c.id != None) # noqa: E711 + .order_by(lambda c: c.pinned_at, "desc", nulls="last") + .order_by(lambda c: c.updated_at, "desc") + ) + + CASES: list[tuple[str, Callable[[dict[str, type]], Any], str]] = [ ("query_user_compound_v7", _q_user_compound, "User"), ("query_user_not_leaf_v7", _q_not_leaf, "User"), @@ -277,6 +296,7 @@ def _q_m2m_exists(m: dict[str, type]) -> Any: ("query_transaction_traversed_record_v7", _q_traversed_record, "Transaction"), ("query_transaction_aggregate_v7", _q_aggregate, "Transaction"), ("query_transaction_global_aggregate_v7", _q_global_aggregate, "Transaction"), + ("query_card_nulls_v7", _q_card_nulls, "Card"), ]