docs: add ENGINE_REFACTORING.md, an R-numbered structural debt log#32
Merged
Conversation
Companion to SQL_PARITY.md. The two tracks answer different questions: P-numbers ask "do we return the right answer?", R-numbers ask "can we keep changing the engine safely?". The engine grew organically, which is why it does as much as it does. The cost is that some structure was never designed, only accreted, and that bill arrives when we try to go further -- richer functions, deeper nesting, correlated scoping. This log exists to make that debt legible so it can be paid incrementally during feature work, and to distinguish "awkward" from "blocking". It is explicitly not a rewrite proposal: principle 1 is that every entry reduces to independently shippable steps. Seeded with seven findings established while scoping parity P3, each with evidence rather than opinion: - R1 two FROM representations (partial; migration deferred as larger than P3 itself, and blocked on real AST work in TableSource) - R2 446 hand-rolled SqlExpression matches across 40 files (partial; walk.rs landed, transformer migration outstanding) - R3 catch-all arms swallow new variants -- only 5 match sites in the whole tree are compiler-guarded, verified by probe - R4 transformer fixtures use ASTs the parser never produces, which is precisely why R1's desync survived a green suite - R5 dead code accumulates undetected - R6 CorrelatedSubqueryAnalyzer unwired, its tests hollow - R7 subqueries are substituted not evaluated -- the structural root cause under parity P3 Adds a sequencing diagram making the load-bearing dependency explicit: the advanced work is gated on R2/R3, because every new expression variant currently costs a 40-file hand audit with silent failure as the default outcome. Cross-links P3 to R7/R2/R6, and surfaces both books of work in CLAUDE.md so future sessions find them before touching the engine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs only — no code. Sits on its own branch so it doesn't restart CI on #31.
What
A companion to
SQL_PARITY.md. The two tracks answer different questions:SQL_PARITY.md(P-numbers)ENGINE_REFACTORING.md(R-numbers)A parity gap is a wrong result. An R-finding is a place where the engine's shape makes the next change disproportionately expensive or risky.
Why now
The engine grew organically and on demand, which is why it does as much as it does. The cost is that some structure was never designed, only accreted — and that bill arrives exactly when we want to go further (richer functions, deeper nesting, correlated scoping).
This is deliberately not a rewrite proposal. Principle 1 in the doc is that every entry must reduce to independently shippable steps. The point is to distinguish "this is awkward" from "this is blocking", and to record why something was deferred so the decision doesn't get relitigated.
Seeded findings
All seven came out of scoping parity P3, and each carries evidence rather than opinion:
SqlExpressionmatches across 40 filesCorrelatedSubqueryAnalyzerunwired, tests hollowTwo worth calling out:
Existsvariant showed only five match sites in the entire codebase fail to compile. Everything else would have silently ignored it.from_source: None, an input the parser never produces. That's the kind of thing worth writing down once so it isn't rediscovered.R1 records the deferral as explicitly as the work: the full FROM migration is blocked on
TableSourcelacking a table-function variant and being unable to carry an alias, which dragsJoinClausein. Assessed as larger than P3 itself.Sequencing
The doc makes the load-bearing dependency explicit — the advanced work is gated on R2/R3, because every new expression variant currently costs a 40-file hand audit with silent failure as the default outcome.
Also
SQL_PARITY.md, and adds the reverse pointer.CLAUDE.mdso future sessions find them before touching the engine.🤖 Generated with Claude Code