refactor(parser): add exhaustive SqlExpression traversal helpers (P3 groundwork, step 1)

[TimelordUK]

Step 1 of the P3 (correlated subqueries) groundwork, following #30. Purely additive — nothing calls these yet, so there is no behaviour change. Migrating the ~10 query_plan transformers onto them is a separate PR.

Why

There was no traversal abstraction at all: 446 SqlExpression::<Variant> patterns across 40 files, every consumer hand-rolling its own match over 24 variants.

Compare ilike_to_like_transformer::transform_expression with cte_hoister::hoist_from_expression — the BinaryOp / FunctionCall / CaseExpression arms are byte-for-byte identical apart from the method name. Each has one real rule wrapped in ~50 lines of boilerplate.

The copy-paste is already causing silent bugs, because every hand-rolled walker ends in _ => {}:

• expression_lifter::extract_column_references misses MethodCall, ChainedMethodCall, Unnest, InSubquery, and both tuple subquery variants.
• having_alias_transformer::collect_aggregates_in_having handles only FunctionCall, BinaryOp, Not — an aggregate inside a CASE in a HAVING clause is never found.

What

map_children (owned rewrite) and visit_children / visit_all (borrowing collector), both with no catch-all arm.

Three design decisions worth reviewing

1. Direct children only; callers drive recursion. This is what collapses a transformer to its one real rule plus a delegate:

match expr {
    SqlExpression::BinaryOp { left, op, right } if op == "ILIKE" => { /* the rule */ }
    other => walk::map_children(other, |e| self.transform(e)),
}

2. Scope boundary = walker boundary. Subquery statements are not descended into. A SELECT alias from the outer query must not be expanded inside a subquery with its own FROM — which is why where_alias_expander gets this right today only by omission (it has no subquery arms at all). Auto-descending would have turned that accident into a real regression.

Same-scope operands of those variants are still visited: InSubquery's expr and InSubqueryTuple's exprs belong to the enclosing query; only the subquery is skipped. Callers that genuinely want to cross the boundary (the CTE hoister) match those variants explicitly first.

3. WindowSpec::order_by IS descended into. It holds real SqlExpressions (partition_by is only Vec<String>), and every hand-rolled walker in the tree misses them today. Two tests cover it. This is the one place where step-2 migration could change a transformer's behaviour — I'll verify that per-transformer rather than assuming it's a pure refactor.

Verification of the core claim

I temporarily added the Exists { negated, subquery } variant P3 will need. Both walk.rs match sites fail with E0004: non-exhaustive patterns.

Revealing side result — only three other matches in the whole codebase are compiler-guarded today:

src/sql/parser/formatter.rs:139
src/sql/parser/formatter.rs:668
src/sql/recursive_parser.rs:569

Everything else has a catch-all and would have silently ignored the new variant. That is the gap this module is meant to close.

Testing

• cargo test: 680 passed (+8 new). One pre-existing failure, history_protection_integration, unrelated — sets APPDATA to a tempdir but still loads the real roaming history.
• DuckDB parity --check: contract holds unchanged at 64 AGREE / 1 DIFFER / 7 GAP. (Guaranteed, since no production path changed — run as insurance.)
• cargo fmt --check clean.

8 unit tests cover CASE branches, nested function args, BETWEEN operands, window ORDER BY (both read and rewrite), the subquery boundary in both directions, and the leaf no-op. They parse real SQL rather than hand-building ASTs.

🤖 Generated with Claude Code