feat: a CTE is a table you can join, and the plumbing is written once - #197
Merged
Merged
Conversation
…tten once `.with()` returned the same builder type it was called on, so the CTE name was not a table anyone could join. `bench/src/scenarios.ts` reached for `"recent_posts" as never` and an `any` callback to measure the shape at all — an escape hatch standing where a type should be, in the repository's own benchmark. It is gone; the scenario now joins the CTE like any other table. `.with(name, query)` adds the name to the schema with the query's row type, so the join proxies carry the CTE's columns and a column it does not project is a compile error. A bare `SelectNode` still works and widens that row to `Record<string, unknown>`, because there is nothing to infer from one. The compiled-query plumbing was written out nine times across five builders — compile an AST this builder already produced, hand back a compiled query wired to run. `compileNode` and `compiledFor` do both, and the builders are 126 lines lighter for it. The ninth copy is where the executor gets forgotten, which is how MERGE came to have none. Type escapes went from 42 to 44: the CTE overload's implementation signature needs two, which is what writing an overload in TypeScript costs. Worth saying rather than rounding down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAwiBVhLmNhWjC9Ro6UMpb
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.
Two pieces of the debt I listed as still open.
The CTE footgun
.with()returned the same builder type it was called on, so the CTE name was not a table anyone could join. The repository's own benchmark reached for the escape hatch to measure the shape at all:An
as neverand ananystanding where a type should be, in the file that publishes our numbers.Now:
The CTE's row type reaches the join proxies, and a column it does not project is a compile error — pinned with
@ts-expect-error. A bareSelectNodestill works and widens that row toRecord<string, unknown>, because there is nothing to infer from one.The plumbing
Compile an AST this builder already produced, hand back a compiled query wired to run: written out nine times across five builders.
compileNodeandcompiledFordo both, and the builders are 126 lines lighter.The ninth copy is where the executor gets forgotten, which is exactly how MERGE came to have none (#196).
Honest accounting
Type escapes went 42 → 44. The CTE overload's implementation signature needs two, which is what writing an overload in TypeScript costs. I said I would reduce them and they went up by two; worth saying rather than rounding down.
pnpm testgreen: 3326 tests, lint clean, types clean, build clean.