Isolate compiled SQL fragment caches by compiler instance - #6997
Conversation
🦋 Changeset detectedLatest commit: af685f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Important
The PR title and commit message say the cache isolation is implemented, but the diff only adds the failing regression test. The source fix in packages/effect/src/unstable/sql/Statement.ts is still missing, so CI fails as expected. Please add the compiler-instance-scoped cache before merging.
Reviewed changes
-
Added a regression test in
packages/effect/test/unstable/sql/Statement.test.tsthat demonstrates the bug: compiling the same fragment first with a PostgreSQL compiler and then with a SQLite compiler returns PostgreSQL placeholders ($1) from the global fragment cache. -
The test currently fails against
mainbecauseCompilerProto.compilestores compiled results under globalSymbol.forkeys on the fragment itself, so every compiler instance shares the same cache slot.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found. The compiler-instance cache isolation is implemented correctly and the regression test passes.
Reviewed changes
- Replaced the global
Symbol.forcache slots onFragmentwith per-compilerWeakMap<Fragment, CompiledStatement>instances (statementCacheandstatementCacheNoTransform) inpackages/effect/src/unstable/sql/Statement.ts. - Updated
CompilerProto.compileto read from and write to the compiler-scoped cache, preserving the existingplaceholderOverridebypass. - The regression test in
packages/effect/test/unstable/sql/Statement.test.tsnow passes: compiling the same fragment with PostgreSQL and SQLite compilers returns the correct dialect-specific placeholders ($1vs?). - Added a concise
effectpatch changeset at.changeset/isolate-sql-compiler-cache.md.
The prior review thread about the missing implementation has been resolved.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|

Summary
Compiling the same fragment with a second compiler can return SQL and placeholders generated by the first compiler's dialect.
Closes EFF-434
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Fragment compilation cache crosses SQL dialects
Module:
StatementAudit ID:
unstable-services-statement-compiler-cacheSeverity / confidence: high / high
What happens
Compiling the same fragment with a second compiler can return SQL and placeholders generated by the first compiler's dialect.
Why it happens
The compiled tuple is cached directly on the fragment under a global symbol per transform mode, without isolating entries by compiler instance.
Expected behavior
Compiler.compile must use the receiving compiler's dialect, identifier transform, placeholders, and custom handlers.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/unstable/sql/Statement.ts:815-830packages/effect/src/unstable/sql/Statement.ts:1032-1036View problematic code at
packages/effect/src/unstable/sql/Statement.ts:815-830View exact lines on GitHub
View problematic code at
packages/effect/src/unstable/sql/Statement.ts:1032-1036View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/unstable/sql/Statement.test.tsObserved failure: FAIL: SQLite compilation returned PostgreSQL SQL with a $1 placeholder.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/unstable/sql/Statement.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715funstable-services-statement-compiler-cache