fix(sql): render NULLS FIRST/LAST for the orderBy nulls option

[wasaybuilds]

Linked issue

Fixes #29932

Summary

OrderByOptions.nulls typechecked at the call site and was discarded at build time, so a sort that reads as explicitly NULL-ordered in the source compiled clean and returned rows in a different order, with nothing raising. The root cause is the one the reporter guessed: OrderByItem had no slot for a NULL placement, so the builder option had nothing to write to and no target ever emitted a NULLS clause.

Of the two closes the issue offers, this takes the "wire it up" one: OrderByItem gains a nulls slot, resolveOrderBy passes the option through on both overloads, and the Postgres and SQLite adapters render it — select lists, window ORDER BY, and aggregate ORDER BY alike.

Omitting the option emits no clause, so existing plans stay byte-identical (asserted per adapter, not just claimed). reverse() flips the placement along with the direction, since inverting a total order has to move NULLs to the opposite end for a backward cursor page to mirror the forward one.

One detail worth flagging for reviewers: the two targets' defaults disagree. PostgreSQL ranks NULLs highest (last under asc); SQLite ranks them lowest (first under asc). That divergence is the argument for the option, so it is documented on the NullsPlacement type and in the skill reference rather than left implicit.

Testing performed

• pnpm --filter @internal/sql-relational-core test — 499 pass (6 new: placement carried, preserved across rewrite, flipped by reverse, double-reverse round trip)
• pnpm --filter @internal/sql-builder test — 174 pass (3 new: nulls reaches the AST on both orderBy overloads, absent when unset)
• pnpm --filter @internal/adapter-postgres test / @internal/adapter-sqlite — exact-SQL assertions for NULLS FIRST / NULLS LAST, the no-clause case, and a window-function ORDER BY
• pnpm --filter @internal/sql-orm-client test — 771 pass, unchanged (largest downstream consumer of order items)
• New order-by-nulls.integration.test.ts against PGlite: seeds the issue's own repro rows ('b', NULL, 'a', NULL, 'c') and asserts real returned row order for overridden and default placement in both directions
• pnpm lint:skills, and biome format + biome check via the pre-commit hook on every touched file

Two things I could not get a clean local signal on, both pre-existing on an unmodified main in my environment (verified by stashing):

• @prisma/orm-* shell packages fail to build with dropped "…" (ambiguous between …) barrel-aggregation errors, which cascades into e2e-tests / integration-tests typecheck. Unrelated to this change, but it means I could not run pnpm typecheck end to end.
• render-typescript.roundtrip and db-init-update.cli suites fail in both adapters. Same failures before and after this change; they look like Windows path/line-ending artifacts on my machine.

Repo-wide pnpm lint is also unusable for me because core.autocrlf=true yields a CRLF working tree in an LF repo, which flags files I never touched. The per-file biome check in the pre-commit hook did pass on this diff.

Skill update

Added the nulls option to skills/prisma-8/references/queries-postgres.md next to the existing db.sql .orderBy(..., { direction }) example, including the differing per-target defaults. No CLI, config, or error-code surface changed.

Checklist

• All commits are signed off (git commit -s) per the DCO.
• I read CONTRIBUTING.md and the change is scoped to one logical concern.
• Tests are updated.
• The PR title is in TML-NNNN: <sentence-case title> form. I have no Linear ticket as an external contributor, so I used the conventional-commit title CONTRIBUTING.md asks external contributors for. Happy to retitle to whatever a maintainer prefers.
• The Skill update section above is filled in.

Notes for the reviewer

Deliberately deferred, since CONTRIBUTING.md asks for one logical change per PR: the ORM lane still has no nulls surface (p.nullable.desc({ nulls: 'last' })). The AST slot this PR adds is what such a surface would write to, so it can land as a follow-up — happy to open one if you want it.

The issue's alternative resolution was to delete nulls from OrderByOptions so the gap fails at compile time instead. I went the other way because the type is already published and the capability is a plain ORDER BY feature both targets support (SQLite since 3.30). Say the word if you would rather have the removal.

Also worth a reviewer's eye: reverse() now flips the NULL placement. Nothing in-tree sets nulls on a reversed item today, so it is not observable yet, but it is the semantics cursor pagination will need once the ORM lane exposes the option.

Summary by CodeRabbit

• New Features

  • Added explicit NULLS FIRST and NULLS LAST ordering for SQL queries.
  • Supports top-level, aggregate, and window-function ORDER BY clauses.
  • Preserves database defaults when no null placement is specified.
  • Reversing sort direction automatically flips explicit null placement.
  • Supports null ordering for both column-based and expression-based sorting.

• Documentation

  • Documented null ordering options and PostgreSQL/SQLite default behavior.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 59a3dbd1-10c9-4a84-9790-9188072b74a1

📥 Commits

Reviewing files that changed from the base of the PR and between 3af065e and 9be9707.

📒 Files selected for processing (10)

• packages/2-sql/4-lanes/relational-core/src/ast/types.ts
• packages/2-sql/4-lanes/relational-core/test/ast/order.test.ts
• packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts
• packages/2-sql/4-lanes/sql-builder/test/runtime/builders.test.ts
• packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts
• packages/3-targets/6-adapters/postgres/test/adapter.test.ts
• packages/3-targets/6-adapters/postgres/test/migrations/order-by-nulls.integration.test.ts
• packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
• packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
• skills/prisma-8/references/queries-postgres.md

🚧 Files skipped from review as they are similar to previous changes (10)

• packages/3-targets/6-adapters/sqlite/test/adapter.test.ts
• packages/2-sql/4-lanes/sql-builder/test/runtime/builders.test.ts
• packages/3-targets/6-adapters/postgres/test/adapter.test.ts
• skills/prisma-8/references/queries-postgres.md
• packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts
• packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
• packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts
• packages/2-sql/4-lanes/relational-core/src/ast/types.ts
• packages/3-targets/6-adapters/postgres/test/migrations/order-by-nulls.integration.test.ts
• packages/2-sql/4-lanes/relational-core/test/ast/order.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The SQL builder now preserves optional NULL placement in OrderByItem. PostgreSQL and SQLite render NULLS FIRST or NULLS LAST for query, aggregate, and window-function ordering. AST rewrites and reversals preserve or invert placement as appropriate.

Changes

NULL ordering

Layer / File(s)	Summary
AST and builder propagation packages/2-sql/4-lanes/relational-core/src/ast/types.ts, packages/2-sql/4-lanes/relational-core/test/ast/order.test.ts, packages/2-sql/4-lanes/sql-builder/src/runtime/builder-base.ts, packages/2-sql/4-lanes/sql-builder/test/runtime/builders.test.ts	OrderByItem now stores optional NullsPlacement. Builders pass the option through, and tests cover construction, rewriting, reversal, and omission.
PostgreSQL rendering and validation packages/3-targets/6-adapters/postgres/src/core/sql-renderer.ts, packages/3-targets/6-adapters/postgres/test/adapter.test.ts, packages/3-targets/6-adapters/postgres/test/migrations/order-by-nulls.integration.test.ts	PostgreSQL renders explicit NULL placement for query, aggregate, and window-function ordering. Adapter and integration tests cover explicit and default ordering.
SQLite rendering and validation packages/3-targets/6-adapters/sqlite/src/core/adapter.ts, packages/3-targets/6-adapters/sqlite/test/adapter.test.ts	SQLite renders explicit NULL placement for query, aggregate, and window-function ordering. Tests verify both modifiers and omitted placement.
SQL builder ordering documentation skills/prisma-8/references/queries-postgres.md	The reference documents the optional nulls setting and target-specific defaults.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 9be97

The PR preserves existing ordering when NULL placement is omitted and adds explicit NULLS FIRST/LAST behavior for supported ordering paths. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant SQLBuilder
  participant OrderByItem
  participant SQLRenderer
  participant Database
  SQLBuilder->>OrderByItem: pass direction and nulls placement
  OrderByItem->>SQLRenderer: provide order-by AST
  SQLRenderer->>Database: execute ORDER BY with NULLS FIRST or NULLS LAST
  Database-->>SQLBuilder: return ordered results

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files. (1 skipped: 1 unsupported.)	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: rendering NULLS FIRST/LAST for the orderBy nulls option.
Linked Issues check	✅ Passed	The PR preserves nulls in the AST and renders explicit NULLS FIRST/LAST for field and expression orderBy paths, satisfying issue #29932.
Out of Scope Changes check	✅ Passed	The changes remain within the linked issue scope, covering AST wiring, SQL rendering, tests, and documentation for orderBy null placement.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.