Skip to content

docs(parity): file P37/P38, restore skipped examples, triage the smoke suite - #63

Merged
TimelordUK merged 1 commit into
mainfrom
docs/p37-p38-and-example-triage
Sep 2, 2026
Merged

TimelordUK merged 1 commit into
mainfrom
docs/p37-p38-and-example-triage

Conversation

@TimelordUK

Copy link
Copy Markdown
Owner

Why

Examples smoke failures were being cleaned up by deleting or editing the offending SQL. That erased the evidence: three statements removed in 2240b73 / 676309f were each demonstrating a real, open engine gap. This restores them behind the statement-level -- [SKIP] directive so the queries stay in the tree as end-to-end acceptance tests without reddening the run.

No Rust changed.

Restored and marked

Example Gap Tracked as
chemistry.sql OR in JOIN ... ON P27 (open)
jsonl_logs.sql ORDER BY ... NULLS LAST P13 stage 2 (open)
statistical_analysis.sql scalar subquery in a CTE P38 (new)

-- [SKIP] is checked in non_interactive.rs before parsing, so unparseable SQL behind it costs nothing. Each P* entry gained an Acceptance test line pointing at the example, so whoever picks the fix up knows where the end-to-end check lives.

For jsonl_logs.sql the NULLS-free statement still runs and the original sits beside it skipped — the demo and the tracked gap, both. Also reverts an unintended whitespace change from 2240b73.

Two new findings

Both have corpus cases; both reference behaviours were confirmed against DuckDB rather than assumed.

P37 — a window function inline in WHERE silently returns zero rows. Success exit code, header printed, no error, on a 20-row table. DuckDB rejects the query outright (Binder Error: WHERE clause cannot contain window functions). Bucketed OURS_ONLY because we "succeed" where the reference errors — the bucket is right, the behaviour is not.

It is a sibling of P15 with one fix site: ExpressionLifter only walks the SELECT list, so an inline window is never hoisted and reaches recursive_where_evaluator.rs as a raw WindowFunction. From QUALIFY that path errors (P15); from WHERE it filters every row out instead. Silent, so it leads the open fix order. Found in expander_rewriters.sql, which advertises this as a working transformation.

P38 — a scalar subquery in a CTE body's SELECT list is never evaluated. Narrower than it first appeared; two controls locate it:

Shape Result
Top-level amount - (SELECT AVG(…)) ✅ AGREE
Same expression inside a CTE body 🚫 GAP
Bare (SELECT AVG(…)) in a CTE, no arithmetic 🚫 error
amount * 2 inside a CTE ✅ AGREE

Neither scalar subqueries nor CTE arithmetic is broken in general — two SELECT-list evaluation paths disagree on which expression types they support. The entry flags checking ENGINE_REFACTORING.md before adding the arm twice.

Smoke suite triage: 13 failures → 1

Twelve examples can never run in CI — they need a live server, post to placeholder hosts (api.example.com, tradesapi, trade-server), or are nvim-plugin macro templates (one contains no runnable statement at all). All now carry file-level -- [TEST:SKIP] <reason>, which test_examples.py::should_skip_file reads and reports.

expander_rewriters is deliberately left failing — it is P26, a real gap, and skipping it would bury it. Smoke failures are already non-blocking (the suite exits 0), so CI stays green while the bug stays visible.

Verification

  • Parity contract holds at 172 cases, 141 AGREE (up 2 — the new controls).
  • Examples: 152/153 pass, all 33 formal tests pass, suite exits 0.
  • No Rust changed, so no cargo fmt / clippy delta.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PpidyXocQt835yaJpKBSHk

…e suite

Examples smoke failures were being cleaned up by deleting or editing the
offending SQL. That erased the evidence: three statements removed last night
were each demonstrating a real, open engine gap. Restore them behind the
statement-level `-- [SKIP]` directive instead, so the queries stay in the tree
as end-to-end acceptance tests without reddening the run.

Restored and marked:
  - chemistry.sql            OR in JOIN ... ON            (P27, open)
  - jsonl_logs.sql           ORDER BY ... NULLS LAST      (P13 stage 2, open)
  - statistical_analysis.sql scalar subquery in a CTE     (P38, new)

Two new findings, both with corpus cases and DuckDB reference behaviour
confirmed:

  P37 - a window function inline in WHERE returns ZERO rows, with a success
        exit code and no error. DuckDB rejects the query outright. Silent
        wrong answer, so it leads the open fix order. Shares a fix site with
        P15 (ExpressionLifter only walks the SELECT list), so the two are one
        piece of work. Found in expander_rewriters.sql, which advertises this
        as a working transformation.

  P38 - a scalar subquery in a CTE body's SELECT list is never evaluated.
        Narrower than it first appeared: two controls show the same expression
        works at the top level, and that plain arithmetic works inside a CTE.
        Two SELECT-list evaluation paths disagree on which expression types
        they support - check ENGINE_REFACTORING.md before adding the arm twice.

Triage of the smoke suite: 13 failures -> 1. Twelve examples need a live
server, post to placeholder hosts, or are nvim-plugin macro templates that
contain no runnable SQL; all are marked with file-level `-- [TEST:SKIP]` plus
the reason. expander_rewriters is deliberately left failing - it is P26, a real
gap, and skipping it would bury it. Smoke failures are already non-blocking, so
CI stays green while the bug stays visible.

Parity contract holds at 172 cases (141 AGREE). All 33 formal example tests
pass. No Rust changed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01PpidyXocQt835yaJpKBSHk
@TimelordUK
TimelordUK merged commit e706847 into main Sep 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant