feat(query): order_by nulls placement e2e + golden vector

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<QueryIrPayload> =
+            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 =

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"
+      }
+    }
+  }
+}

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"

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]

tests/test_query_wire_vectors.py

@@ -76,13 +76,21 @@ 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,
         "Account": Account,
         "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"),
 ]