Skip to content

Route params and literals through a Dialect - #1

Merged
Makisuo merged 3 commits into
mainfrom
feat/dialect-seam
Oct 3, 2026
Merged

Makisuo merged 3 commits into
mainfrom
feat/dialect-seam

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

First steps toward writing SQL for more than one database from the same builder. The builder, tenant-scope analysis and row decoding stay shared; a Dialect holds what differs. The plan for the rest is in design/dialects.md.

What changes

Params go through the dialect. A dialect either writes param values into the SQL as literals (inline, the ClickHouse default) or leaves placeholders and returns the encoded values in a new CompiledQuery.parameters field (bind, for $1/? drivers). Params now resolve once at the top of the statement instead of in every nested compile, so placeholders number correctly across unions, subqueries, joins and CTEs. Bound values are the column codec's wire form, the same value an inline literal is written from.

Literals go through the dialect. Dialect.quoteString and Dialect.literal write every string fragment, column literal and inline param. Literals are written inside the query callbacks, which take no dialect argument, so compile installs the dialect for the length of the compile (the same save/restore pattern withSubqueryCompiler uses).

The param-marker guard is enforced, not assumed. Params are resolved by rewriting __PARAM_ placeholders in the finished SQL. Until now, the only thing stopping a value from spelling one was ClickHouse's \x5F escape. A literal that contains the marker now fails the compile with InvalidLiteral, whatever the dialect's escaping.

ClickHouse output

Byte-identical. The exact-SQL suites in compile.test.ts pass unchanged.

API changes

  • compile, compileUnsafe, compileUnion, compileUnionUnsafe take options.dialect.
  • New exports: clickhouseDialect, Dialect, ParamStyle.
  • CompiledQuery.parameters is a new required field, empty under ClickHouse. Code that builds a CompiledQuery object by hand needs parameters: [].
  • compileUnionUnsafe no longer accepts the internal enclosingCtes option. Its type was never exported.

Not in this PR

Identifier quoting, splitting CHType into a type plus a transport codec, and the Postgres dialect itself (steps 3 to 6 in the design doc). Identifiers, clauses and functions are still ClickHouse SQL.

Testing

  • tsc for the library and the tests
  • vitest run src/ch src/sql src/docs-examples.test.ts: 313 passed, including 10 new dialect tests
  • bun run build, then the doc-citation, export-catalog and doc-example checks
  • Not run: the benchmark tests and test:clickhouse (live ClickHouse via Docker)

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features
    • Query compilation now supports selectable SQL dialects, allowing parameters to be rendered as inline values or bound placeholders. Bound values are returned in placeholder order, with placeholder reuse following the selected dialect’s rules.
    • ClickHouse remains the default, rendering parameter values inline. Dialect-specific string and literal formatting is supported, with invalid or missing parameter values reported during compilation.
  • Documentation
    • Added guidance on dialect selection, parameter binding, and how dialect behavior applies to queries, unions, and subqueries.

A dialect decides how resolved params reach the server: written into the
SQL as literals (ClickHouse, the default, byte-identical output) or left as
bind placeholders with the encoded values returned in
CompiledQuery.parameters. Params now resolve once at the top of the
statement so a binding dialect numbers placeholders across unions,
subqueries, joins and CTEs.
Dialect gains quoteString and literal, which write every string fragment,
column literal and inline param. compile installs the dialect for the
length of the compile, since literals are written inside query callbacks
that take no dialect argument. A literal that contains the param marker
now fails with InvalidLiteral instead of relying on every dialect's
escaping to prevent it. ClickHouse output is unchanged.
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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 59 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
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 193d7cde-9843-46d6-b14e-a2fde3379a87
📥 Commits

Reviewing files that changed from the base of the PR and between 2a93784 and ffa089d.

📒 Files selected for processing (10)
  • design/dialects.md
  • docs/params-and-compilation.md
  • docs/reference.md
  • src/ch/compile.ts
  • src/ch/dialect.test.ts
  • src/ch/dialect.ts
  • src/ch/index.ts
  • src/ch/literal.ts
  • src/sql/literal-syntax.ts
  • src/sql/sql-fragment.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 37535433-4e2a-4dd0-a2f2-e86873bca67d

📥 Commits

Reviewing files that changed from the base of the PR and between 14afbeb and 2a93784.

📒 Files selected for processing (10)
  • design/dialects.md
  • docs/params-and-compilation.md
  • docs/reference.md
  • src/ch/compile.ts
  • src/ch/dialect.test.ts
  • src/ch/dialect.ts
  • src/ch/index.ts
  • src/ch/literal.ts
  • src/sql/literal-syntax.ts
  • src/sql/sql-fragment.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Query compilation now supports dialect-specific literal rendering and parameter binding. Compiled queries expose bound values in placeholder order. The default ClickHouse dialect continues to inline parameters, and nested queries and unions defer parameter rendering to the enclosing compilation.

Changes

Dialect-aware compilation

Layer / File(s) Summary
Dialect and literal rendering
src/sql/literal-syntax.ts, src/ch/dialect.ts, src/ch/literal.ts, src/sql/sql-fragment.ts, src/ch/index.ts, design/dialects.md
Adds the Dialect and LiteralSyntax contracts, compile-scoped literal syntax, checked literal rendering, and dialect-aware string quoting. Exports the ClickHouse dialect and dialect types.
Parameter rendering across queries
src/ch/compile.ts, src/ch/dialect.test.ts, docs/params-and-compilation.md, docs/reference.md
Adds dialect options to query and union compilation. Binding dialects produce placeholders and ordered parameters; nested queries and unions defer rendering to the enclosing compilation. Tests and documentation cover binding, reuse, literal checks, and error cases.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant compileCHUnsafe
  participant withDialect
  participant compileInner
  participant renderParams
  participant Dialect
  participant CompiledQuery
  Caller->>compileCHUnsafe: query, params, dialect
  compileCHUnsafe->>withDialect: install dialect for compilation
  withDialect->>compileInner: compile query
  compileInner->>renderParams: SQL and query parameters
  renderParams->>Dialect: encode values and render literals or placeholders
  Dialect-->>renderParams: rendered SQL and encoded bindings
  renderParams-->>compileInner: SQL and bindings
  compileInner-->>CompiledQuery: SQL and parameters
  CompiledQuery-->>Caller: compiled query
Loading

Merge Risk: ⚪ Minimal · up to 2a937

No identified issue needs resolution before merge; normal validation remains appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2a937

The inspected paths preserve value validation and tenant-scope checks while adding explicit protection against parameter-marker substitution. No introduced attack path was established, but alternate database behavior and downstream adoption remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The material exposure is query generation for consumers selecting a dialect, including their nested queries, CTEs, joins, and union branches. Incorrect trusted dialect semantics could affect the SQL those consumers execute; the inspected change does not establish new database credentials, service authority, or a concrete cross-tenant attack path.

Trust Boundaries and Controls

  • observed — Runtime parameter values are schema-encoded before transport. In binding mode they remain outside SQL text; in inline mode dialect output is checked for the reserved marker. Dialect functions themselves are caller-supplied executable authority: marker rejection does not validate arbitrary SQL quoting or database comparison semantics.
  • observed — The same-dialect fast path checks only that literal syntax is installed, not its provenance. However, direct context-installation helpers are absent from the inspected public barrels and declared package export map. An unchecked internal override therefore requires additional code authority; ordinary parameter values do not establish that path.

Resilience and Maintainability Implications

  • observed — Binding accumulation is local to one rendering call, and missing values or unresolved markers throw before a compiled result is returned. Finally-based context restoration also runs when compilation fails, preventing a failed synchronous compilation from leaving its dialect installed for later calls.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: routing parameters and literals through the new Dialect abstraction.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. (3 skipped: 3 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

Comment @coderabbitai help to get the list of available commands.

@Makisuo Makisuo mentioned this pull request Oct 2, 2026
@Makisuo
Makisuo added this pull request to stack #3 October 3, 2026 13:25
@Makisuo
Makisuo merged commit e0cb976 into main Oct 3, 2026
4 checks passed
@Makisuo
Makisuo deleted the feat/dialect-seam branch October 3, 2026 15:34
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