Repository navigation
Route params and literals through a Dialect - #1
Conversation
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.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou'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. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (10)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughQuery 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. ChangesDialect-aware compilation
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
Merge Risk: ⚪ Minimal · up to No identified issue needs resolution before merge; normal validation remains appropriate. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
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
Dialectholds what differs. The plan for the rest is indesign/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 newCompiledQuery.parametersfield (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.quoteStringandDialect.literalwrite every string fragment, column literal and inline param. Literals are written inside the query callbacks, which take no dialect argument, socompileinstalls the dialect for the length of the compile (the same save/restore patternwithSubqueryCompileruses).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\x5Fescape. A literal that contains the marker now fails the compile withInvalidLiteral, whatever the dialect's escaping.ClickHouse output
Byte-identical. The exact-SQL suites in
compile.test.tspass unchanged.API changes
compile,compileUnsafe,compileUnion,compileUnionUnsafetakeoptions.dialect.clickhouseDialect,Dialect,ParamStyle.CompiledQuery.parametersis a new required field, empty under ClickHouse. Code that builds aCompiledQueryobject by hand needsparameters: [].compileUnionUnsafeno longer accepts the internalenclosingCtesoption. Its type was never exported.Not in this PR
Identifier quoting, splitting
CHTypeinto 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
tscfor the library and the testsvitest run src/ch src/sql src/docs-examples.test.ts: 313 passed, including 10 new dialect testsbun run build, then the doc-citation, export-catalog and doc-example checkstest:clickhouse(live ClickHouse via Docker)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit