fix(parity): NULL ordering in ORDER BY — P17 + P13 stage 2

[TimelordUK]

Closes both halves of the ORDER BY comparator's NULL rule in one slice. They
are the same comparator and both decisions were taken together on 2026-08-02, so
this was implementation rather than deliberation.

Parity 141 → 152 AGREE (177 cases, contract holds). Eleven cases closed in
one change — the largest single movement in the effort so far.

What changes for users

⚠️ Behaviour change. The default is now NULLS LAST in both directions.
Previously NULL sorted as the minimum value, so ORDER BY score put NULLs first
and ORDER BY score DESC put them last.

This is a recorded choice, not a bug fix — standard SQL leaves NULL placement
implementation-defined and the major engines genuinely disagree (SQLite/MySQL
treat NULL as smallest, PostgreSQL as largest, DuckDB pins NULLS LAST). We follow
the reference engine, which is the whole point of having one. Anyone relying on
NULLs-first in an ascending sort now writes it explicitly. Called out in
CHANGELOG.md.

✨ NULLS FIRST / NULLS LAST is now parsed and honoured, per ORDER BY item:

SELECT id, score FROM data ORDER BY score DESC NULLS FIRST, id;

Before this the clause was not parsed at all. Until P13 stage 1 it was silently
discarded along with every clause after it, so ORDER BY amount DESC NULLS LAST LIMIT 3 quietly returned every row instead of 3; since stage 1 it has been a
parse error. Both are now gone.

Three things worth reviewing

1. One shared comparator, not two matching edits.
datavalue_compare::compare_for_order_by(a, b, ascending, nulls_first) now
serves both sort sites — DataView::apply_multi_sort and
window_context::compare_by_sort_cols. That shared function, not the corpus
cases, is what stops the two drifting apart again; the cases only cover one shape
each. NULL placement is applied before direction and is never reversed by it —
NULLS LAST means last in the output whichever way the values sort. A comparator
that reversed the NULL arm along with the values would pass every ASC case and
fail the DESC ones, so both directions are asserted.

compare_datavalues is deliberately untouched. It still sorts NULL as the
minimum because it is shared with aggregates, MIN/MAX and TUI column sorting,
where that is not the same question.

2. The window site was worse than the finding recorded.
It was filed as "follows a different NULL rule". In fact it compared DataValues
through their derived PartialOrd, which orders by variant index — and Null
is the last variant, so NULL sorted as the maximum, then got reversed by DESC
into first place. The same derived ordering compared cross-type values by variant
rather than by value, so Integer(100) sorted below Float(1.0) in a window's
internal sort. Routing this site through the shared comparator fixed that too; it
has its own regression test.

3. NULLS, FIRST and LAST are not reserved words.
They are matched contextually, in the one position they can appear. All three are
plausible column names — first and last especially — and reserving them would
break queries that have nothing to do with NULL ordering, in a tool that reads
user CSVs with arbitrary headers. SELECT nulls FROM t ORDER BY nulls still
parses, and there is a test that says so. A malformed clause errors naming the
offending token rather than being ignored, per P13 stage 1's rule.

Also here

• examples/jsonl_logs.sql: the [SKIP] is dropped. Both forms now return the
  identical top 5 — the filtered set contains a row with a NULL latency_ms, so
  NULLS LAST is doing real work there and the default agreeing with it is the
  P17 change visible end-to-end.
• Corpus: two cases shared the id order_by_nulls_first_limit, so --check
  reported one id twice and an id filter would have run both. The NULL-free one
  is now order_by_nulls_first_limit_nullfree.
• The formatters round-trip what was typed, not what was resolved — which is why
  NullsOrder::Unspecified is distinct from Last even though the two mean the
  same thing to the comparator. Printing NULLS LAST onto a query the user never
  wrote it in would be a silent edit.

Not fixed, deliberately

A third comparator exists: csv_datasource.rs::sort_results sorts
serde_json::Values and still places NULLs first. It is unreachable — it hangs
off CsvApiClient, which buffer.rs keeps only "for API compatibility" and never
calls, and the DataSourceAdapter that would reach it has no callers either.
Left as-is rather than fixed blind, and recorded in the P17 entry so that whoever
revives that path knows it needs the same rule. Reviving it without this is a
silent divergence, not a compile error.

R11 (ORDER BY's private column resolver) was kept out of this slice — it is a
separate behaviour change on unquoted dotted names and wants its own parity run.

Verification

• Parity: 152 AGREE / 10 DIFFER / 12 GAP / 1 OURS_ONLY / 2 BOTH_ERR — contract
  holds. All five null_edges.csv acceptance cases flipped, including the sharp
  one (order_by_nulls_first_limit returns exactly ids 3, 10, 11), plus the
  window second site win_first_value_unfiltered.
• cargo test: 749 + 469 pass, 0 failures. New unit tests cover the comparator
  (both directions, missing-cell vs explicit NULL, mixed numerics) and the parser
  (per-item clause, case-insensitivity, non-reserved words, loud error).
• Examples: 152/153, all 33 FORMAL pass. The single failure is
  expander_rewriters query 7, the example deliberately left failing for P37.
• cargo fmt clean; no new clippy findings.

The acceptance criteria mattered here: the three original order_by_nulls_*
cases run on a NULL-free fixture, so an implementation that parsed the clause into
the AST and then ignored it would have flipped all three to AGREE and passed the
gate. The null_edges.csv cases added the day before were the real check.

Docs: docs/SQL_PARITY.md P13 and P17 updated; P37 moves to NEXT in the fix
queue.

🤖 Generated with Claude Code

https://claude.ai/code/session_015fQ6qjYnQjmUAXgaQMk4Qq