Skip to content

feat(compile): install the dialect through Effect context, not module state - #97

Merged
Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/dialect-context
Nov 2, 2026
Merged

Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/dialect-context

Conversation

@artile

@artile artile commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

compile kept its dialect, and the rest of its state, in module-level let slots for the length of a compile. This is safe only while every compile step is synchronous, and the dialect could not come from Effect context.

Closes #64.
Part of #52.

Changes

  • New CurrentDialect, a Context.Reference<Dialect | undefined>, exported from /clickhouse and /postgres. compile uses options.dialect, then CurrentDialect, then the entry's dialect.
  • The /postgres compile functions read CurrentDialect when the Effect runs, not when it is built.
  • New internal helper src/sql/fiber-local.ts. It installs a value in the context of the fiber that runs a synchronous body, with the same save/restore as before. Outside a fiber, it runs the body in a fiber of its own (Effect.runSync).
  • All seven module-level slots on the compile path now use this helper: dialect (ch/dialect.ts), SQL syntax, literal binder, render tracker, subquery compiler, compile failures, and bound literals (ch/compile.ts). The compile path has no module-level mutable state now.
  • compileUnsafe runs in the caller's fiber context (Effect.runSyncWith). A nested unsafe compile keeps the outer compile's state, as before.
  • Tests (src/ch/dialect.test.ts): CurrentDialect is read and options.dialect overrides it; the /postgres default and override; 20 compiles for two dialects in concurrent fibers, with Effect.yieldNow between steps; and a compile in a second fiber, started during a numbered compile, that does not see its dialect. On main, the second-fiber test fails: the second compile writes $1.
  • Docs: docs/params-and-compilation.md (new section, with a test in docs-examples.test.ts), docs/reference.md, design/dialects.md. One minor changeset.

Left out

  • No explicit ctx argument on the render path (Lazy.render, compile(fragment) from /sql). That change touches about 300 call sites and breaks custom fragments. Fiber context gives the same isolation and keeps the public API.
  • Compile is still synchronous. This PR does not add an async step. It only makes sure that an async step later does not break the state.
  • A function from the wrong dialect (assertSet in ch/functions/builtin.ts) is still a QueryBuilderDefect at compile time. Dialect mismatch and missing sources as type errors #66 makes it a type error. That PR can read CurrentDialect.
  • No Database change. Db.run passes its dialect explicitly, as before. Tenant routing to several databases (Database-per-tenant routing from the tenant scope #58) can provide CurrentDialect for each database.
  • MySQL, SQLite and D1 are not part of this PR.

Validation

Run on upstream/main c5fa679 (the branch is rebased; no new upstream commits), with a live ClickHouse 26.8.2.7 (the CI version):
EFFECT_ORM_CLICKHOUSE_URL=http://127.0.0.1:8123 EFFECT_ORM_CLICKHOUSE_USER=maple EFFECT_ORM_CLICKHOUSE_PASSWORD=maple.

  • bun run build: pass.
  • bun run typecheck: pass.
  • bunx vitest run --testTimeout=60000 --hookTimeout=60000: 60 of 60 files pass, 1039 of 1039 tests pass, 0 skipped. (The machine was under load. The longer timeout is for that only.)
  • node scripts/check-doc-citations.mjs: pass (25 files).
  • node scripts/check-exports-documented.mjs: pass (288 /clickhouse, 206 /postgres exports).
  • node scripts/check-doc-examples.mjs: pass (21 files typechecked).
  • bun scripts/check-package.ts: pass (isolated tarball, CommonJS consumer, 13 entries).
  • Compile speed: a micro benchmark (20,000 compileUnsafe runs of a grouped, filtered query) gave 47–58 µs per compile on main and 49–53 µs on this branch. The noise was larger than any difference.
  • Upstream CI for a fork PR waits for a maintainer to approve the run.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3e57accd-49f4-45fc-bc1d-cedf2c1936c1
Commits

Reviewing files that changed from the base of the PR and between b57faf1 and 052c33e.

Files selected for processing: 18
  • .changeset/dialect-in-context.md
  • design/dialects.md
  • docs/params-and-compilation.md
  • docs/reference.md
  • docs/sqlite.md
  • src/ch/compile.ts
  • src/ch/dialect.test.ts
  • src/ch/dialect.ts
  • src/ch/failure.ts
  • src/ch/subquery-context.ts
  • src/core.ts
  • src/docs-examples.test.ts
  • src/postgres.ts
  • src/sql/fiber-local.ts
  • src/sql/literal-binder.ts
  • src/sql/render-tracker.ts
  • src/sql/sql-syntax.ts
  • src/sqlite.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Makisuo and others added 2 commits November 2, 2026 16:28
The /sqlite entry (added on main after this branch) still chose its default
dialect when the call was made and ignored CurrentDialect. It now resolves
options.dialect, then CurrentDialect, then sqliteDialect when the Effect runs,
like /postgres. D1 can be provided once as CurrentDialect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Makisuo

Makisuo commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Merge-readiness pass (maintainer side):

  • Merged current main (SQLite/D1 dialects, DDL, migrations, fuzz, Effect 4.0.6, afterCommit). No conflicts.
  • main's new /sqlite entry still picked its dialect at call time and ignored CurrentDialect. It now resolves options.dialect, then CurrentDialect, then sqliteDialect when the Effect runs, the same way /postgres does (compileUnion is wrapped in Effect.suspend for the same reason). So D1 can be provided once with Effect.provideService(SQLite.CurrentDialect, SQLite.d1Dialect).
  • Checked that nothing on main added module-level compile state. The only module lets left are the custom param-kind counter and the migration ledger sequence, and neither is on the compile path.
  • New test in src/ch/dialect.test.ts: /sqlite defaults to ? placeholders, reads CurrentDialect for compile and compileUnion, and with D1 from context refuses a statement with 101 bindings. Docs updated (docs/sqlite.md, docs/reference.md, docs/params-and-compilation.md).

Verified locally against ClickHouse 26.8: bun run build, bun run typecheck, bun run test (73 files / 1436 tests, plus the doc and export checks), tests/*.clickhouse.test.ts live (9 files / 207 tests), and vitest run tests/fuzz with FUZZ_SCALE=0.3 (9 files / 269 tests). All pass.

Makisuo added a commit to artile/effect-orm that referenced this pull request Nov 2, 2026
…feat/type-level-dialect

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

# Conflicts:
#	src/ch/dialect.ts
@Makisuo
Makisuo merged commit 23ecd11 into MapleTechLabs:main Nov 2, 2026
3 checks passed
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.

Install the dialect through Effect context, not module state

2 participants