Skip to content

refactor(walk): make the boundary-crossing forms the traversal primitives#35

Merged
TimelordUK merged 2 commits into
mainfrom
refactor/walk-crossing
Jul 18, 2026
Merged

refactor(walk): make the boundary-crossing forms the traversal primitives#35
TimelordUK merged 2 commits into
mainfrom
refactor/walk-crossing

Conversation

@TimelordUK

Copy link
Copy Markdown
Owner

Next slice of R2, following #33.

The problem

The three transformers that must cross the subquery scope boundary — CTE hoisting, INTO removal, ILIKE -> LIKE — each hand-listed the five subquery-bearing SqlExpression variants. That arrangement compiles clean and silently stops crossing the day a sixth is added (Exists, for P3). It's the R3 failure mode precisely: #33 fixed two live bugs of exactly that shape (ILIKE inside OVER (ORDER BY ...), INTO inside a tuple IN).

The change

Invert which form is primitive. map_children_crossing / visit_children_crossing take a second closure for the nested statement; map_children is now that with an identity handler and visit_children with a no-op one. The variant list is written down once, in walk.rs — adding one is a compile error in a single place.

hoist_from_expression goes from 37 lines to 6; net −224 lines of copy-pasted match arms.

Two forced signature details

  • Both closures take an explicit ctx instead of capturing. CTEHoister's recursion is &mut self, and two closures each capturing self mutably won't borrow-check — same for &mut deps on the visit path. ctx gives one mutable borrow that the helper splits across the two calls.
  • f_stmt takes and returns the Box, not the statement. By value, the opaque path — most transformers — would unbox, free and realloc a large struct at every subquery. Passing the Box keeps the identity handler a genuine passthrough.

Both are documented in the module docs and the R2 entry.

Verification

Behaviour is unchanged — all three transformers already crossed the boundary; this only moves where the variants are listed. New tests pin the property the split exists to provide: the variants map_children treats as opaque must be reachable through the crossing forms, and the tuple forms must yield both their same-scope operands and their statement.

  • cargo test: 685 unit tests pass; cargo fmt --check and clippy clean on the touched files
  • Checked end-to-end that ILIKE inside an IN subquery still matches case-insensitively

Local Windows runs show ~21 failures in the Python/examples/history suites; all are environmental (missing .exe suffix, cp1252 decoding of the CLI's stdout, a #[cfg(unix)]-only HOME redirect) and all are green in CI. Filed separately from this work.

🤖 Generated with Claude Code

TimelordUK and others added 2 commits July 18, 2026 18:07
The three transformers that must cross the subquery scope boundary (CTE
hoisting, INTO removal, ILIKE -> LIKE) each hand-listed the five
subquery-bearing SqlExpression variants. That compiles clean -- and
silently stops crossing -- the day a sixth is added, which is exactly the
R3 failure mode: PR #33 fixed two live bugs of that shape.

Invert it. map_children_crossing / visit_children_crossing take a second
closure for the nested statement and are now the primitives;
map_children is that with an identity handler, visit_children with a
no-op one. The variant list exists once, in walk.rs, so adding one is a
compile error in a single place.

Two things the signature is forced into:

- Both closures take an explicit ctx rather than capturing. CTEHoister's
  recursion is &mut self, and two closures each capturing self mutably
  will not borrow-check; the same applies to &mut deps on the visit path.
  ctx gives one mutable borrow that the helper splits across the calls.
- f_stmt takes and returns the Box, not the statement. By value, the
  opaque path -- which is most transformers -- would unbox, free and
  realloc a large struct at every subquery. Passing the Box keeps the
  identity handler a true passthrough.

Behaviour is unchanged; the three transformers already crossed. Tests
pin the property the split exists to provide: the variants
map_children treats as opaque must be reachable via the crossing forms,
and the tuple forms must yield both their same-scope operands and their
statement.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@TimelordUK
TimelordUK merged commit 36472a1 into main Jul 18, 2026
8 checks passed
TimelordUK added a commit that referenced this pull request Jul 18, 2026
docs(refactoring): refresh R2 status after #35
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