From ec3d77e492341095411a829a310a408c27981135 Mon Sep 17 00:00:00 2001 From: Richard Wooding Date: Wed, 9 Sep 2026 17:53:50 +0200 Subject: [PATCH] feat: add WithPlaceholderStyle for callers that rebind placeholders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each dialect renders its native placeholder: $1 for PostgreSQL and DuckDB, @p1 for BigQuery, ? for MySQL, SQLite and Spark. That leaves no way to serve a caller whose driver or query builder does the numbering itself. GORM, sqlx.Rebind and squirrel each number placeholders from their own running count across the whole statement and then rewrite them for the target driver. A fragment arriving already numbered is passed through untouched — GORM's clause.Expr.Build reacts only to ?, copying every other byte verbatim — so the $1 reaches PostgreSQL unbound. Until now such callers had to wrap the dialect to override WriteParamPlaceholder. WithPlaceholderStyle(PlaceholderQuestion) emits ? for every parameter and leaves the numbering to them. The style is orthogonal to the dialect: it changes only the placeholder syntax, never the SQL around it, and is a no-op for the three dialects whose native placeholder is already ?. Implemented as a converter field rather than a dialect decorator, on purpose. A struct embedding the Dialect interface has only that interface's method set, so a wrapper would not satisfy dialect.IndexAdvisor and AnalyzeQuery would silently stop recommending indexes. The five identical increment-write-append triples in visitConst collapse into one writeParamPlaceholder helper, so all five paths behave alike by construction. PlaceholderDialect is the default and output is unchanged for every existing caller — the existing parameterized tests pass with no expectation edits. Rewriting $n to ? after conversion is not a safe substitute: matches() inlines its pattern as a string literal, so matches('a$1b') puts a literal $1 in the SQL that such a rewrite would corrupt. Only the converter knows which $1 it wrote. One expression is rejected rather than silently mis-bound. PostgreSQL spells jsonb existence as the ? operator, so has() on a JSONB column emits a ? that a placeholder-scanning consumer cannot tell from a bind marker; it would bind a value to the operator and shift every parameter after it. Conversion now fails when the ? count exceeds the parameter count. Only PostgreSQL is affected — every other dialect writes JSON existence as a function call. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 25 +++++ README.md | 25 ++++- cel2sql.go | 155 +++++++++++++++++++++------ docs/parameterized-queries.md | 89 +++++++++++++++ placeholder_style_test.go | 196 ++++++++++++++++++++++++++++++++++ 5 files changed, 459 insertions(+), 31 deletions(-) create mode 100644 placeholder_style_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 850b121..f9b9435 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ ## [Unreleased] +### Added +- **`WithPlaceholderStyle` option** — render `?` for every bind parameter instead + of the dialect's native placeholder, for callers whose driver or query builder + rebinds placeholders itself (GORM, `sqlx.Rebind`, squirrel). Those number + placeholders from their own running count across the whole statement, so a + fragment that arrives already numbered as `$1, $2` is passed through untouched + and reaches the database unbound. `PlaceholderDialect` remains the default and + output is unchanged for every existing caller; the style is orthogonal to the + dialect and is a no-op for MySQL, SQLite and Spark, whose native placeholder is + already `?`. `WithParamStartIndex` has no visible effect under + `PlaceholderQuestion` — the two options serve opposite situations. + + Rewriting `$n` to `?` after conversion is not a safe substitute: `matches()` + inlines its pattern as a string literal, so `matches('a$1b')` puts a literal + `$1` in the SQL that such a rewrite would corrupt. + + **Known limitation**: PostgreSQL spells jsonb existence as the `?` operator, so + `has()` on a JSONB column would emit a `?` that a placeholder-scanning consumer + cannot distinguish from a bind marker. `ConvertParameterized` returns an error + in that case rather than SQL that would silently bind wrong. Only PostgreSQL is + affected; every other dialect writes JSON existence as a function call. Emitting + `jsonb_exists(...)` under this style would lift the restriction, but the + operator form is what the planner reliably matches against a GIN index, so that + is a performance trade rather than a free win. + ## [3.9.3] - 2026-09-09 ### Changed - **Dependencies**: 22 grouped minor/patch Go module bumps (#190) — diff --git a/README.md b/README.md index a0b0df9..c758794 100644 --- a/README.md +++ b/README.md @@ -175,11 +175,14 @@ sql, err := cel2sql.Convert(ast, cel2sql.WithDialect(spark.New())) | Arrays | `ARRAY[...]` | JSON arrays | JSON arrays | `[...]` | `[...]` | `array(...)` | | Array index | 1-indexed | n/a | n/a | 1-indexed | 0-indexed (`OFFSET`) | 0-indexed | | UNNEST | `UNNEST(x)` | `JSON_TABLE(...)` | `json_each(x)` | `UNNEST(x)` | `UNNEST(x)` | `EXPLODE(x)` | -| Param placeholder | `$1, $2` | `?, ?` | `?, ?` | `$1, $2` | `@p1, @p2` | `?, ?` | +| Param placeholder [^ph] | `$1, $2` | `?, ?` | `?, ?` | `$1, $2` | `@p1, @p2` | `?, ?` | | Timestamp cast | `TIMESTAMP WITH TIME ZONE` | `DATETIME` | `datetime()` | `TIMESTAMPTZ` | `TIMESTAMP` | `TIMESTAMP` | | Contains | `POSITION()` | `LOCATE()` | `INSTR()` | `CONTAINS()` | `STRPOS()` | `LOCATE()` | | Index analysis | BTREE, GIN, GIN+trgm | BTREE, FULLTEXT | BTREE | ART | CLUSTERING, SEARCH_INDEX | not supported in v1 | +[^ph]: The default. `WithPlaceholderStyle(PlaceholderQuestion)` emits `?` for any +dialect — see [Placeholder Style](#placeholder-style). + ### Per-Dialect Type Providers Each dialect has its own type provider for mapping database types to CEL types. All providers support both pre-defined schemas (`NewTypeProvider`) and dynamic schema loading (`LoadTableSchema`): @@ -312,6 +315,26 @@ rows, err := db.Query( ) ``` +### Placeholder Style + +Each dialect renders its native placeholder by default (`$1` for PostgreSQL and +DuckDB, `?` for MySQL/SQLite/Spark, `@p1` for BigQuery). Callers whose driver or +query builder rebinds placeholders itself — GORM, `sqlx.Rebind`, squirrel — need +`?` regardless of dialect, because they number placeholders from their own count +across the whole statement and cannot renumber a fragment that arrives numbered: + +```go +result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) +// result.SQL: "name = ? AND age > ?" + +db.Where(result.SQL, result.Parameters...).Find(&users) // GORM +``` + +See [docs/parameterized-queries.md](docs/parameterized-queries.md#placeholder-style) +for why rewriting `$n` yourself is unsafe, and for the one PostgreSQL expression +(`has()` on a JSONB column) this style rejects. + ### What Gets Parameterized? **Parameterized** (values become placeholders): diff --git a/cel2sql.go b/cel2sql.go index 0e9918f..cb0bbd1 100644 --- a/cel2sql.go +++ b/cel2sql.go @@ -51,15 +51,16 @@ type ConvertOption func(*convertOptions) // convertOptions holds configuration options for the Convert function. type convertOptions struct { - schemas map[string]schema.Schema - jsonVars map[string]bool // Variable names that are JSONB columns - columnAlias map[string]string // CEL variable name → SQL column name - ctx context.Context - logger *slog.Logger - maxDepth int // Maximum recursion depth (0 = use default) - maxOutputLen int // Maximum SQL output length (0 = use default) - dialect dialect.Dialect // SQL dialect (nil = PostgreSQL default) - paramStartIndex int // First placeholder index for ConvertParameterized (1 = $1; 0 means default 1) + schemas map[string]schema.Schema + jsonVars map[string]bool // Variable names that are JSONB columns + columnAlias map[string]string // CEL variable name → SQL column name + ctx context.Context + logger *slog.Logger + maxDepth int // Maximum recursion depth (0 = use default) + maxOutputLen int // Maximum SQL output length (0 = use default) + dialect dialect.Dialect // SQL dialect (nil = PostgreSQL default) + paramStartIndex int // First placeholder index for ConvertParameterized (1 = $1; 0 means default 1) + placeholderStyle PlaceholderStyle // How ConvertParameterized renders placeholders } // WithDialect sets the SQL dialect for conversion. @@ -233,6 +234,54 @@ func WithParamStartIndex(index int) ConvertOption { } } +// PlaceholderStyle selects how ConvertParameterized renders bind placeholders. +type PlaceholderStyle int + +const ( + // PlaceholderDialect uses the dialect's native placeholder syntax: $1 for + // PostgreSQL and DuckDB, @p1 for BigQuery, ? for MySQL, SQLite and Spark. + // This is the default. + PlaceholderDialect PlaceholderStyle = iota + + // PlaceholderQuestion emits ? for every parameter regardless of dialect, + // leaving the numbering to the caller. + // + // Use it with a driver or query builder that rebinds placeholders itself — + // GORM, sqlx.Rebind, squirrel. Those number placeholders from their own + // running count across the whole statement, so they cannot bind a fragment + // that arrives already numbered, and they have no way to renumber one. + // + // Rewriting $1..$n to ? after the fact is not a safe substitute: a pattern + // passed to matches() is inlined as a string literal, so a CEL expression + // like matches('a$1b') puts a literal $1 in the SQL that such a rewrite + // would corrupt. Only the converter knows which $1 is a placeholder. + PlaceholderQuestion +) + +// WithPlaceholderStyle sets how ConvertParameterized renders bind placeholders. +// It has no effect on Convert, which inlines literals rather than binding them. +// +// The style is orthogonal to the dialect: it changes only the placeholder +// syntax, never the SQL around it. For MySQL, SQLite and Spark — whose native +// placeholder is already ? — PlaceholderQuestion produces identical output. +// +// WithParamStartIndex has no visible effect under PlaceholderQuestion, since +// there is no index to offset. The two options serve opposite situations: +// WithParamStartIndex for splicing into a query you number yourself, +// PlaceholderQuestion for handing the numbering to a driver. +// +// Example: +// +// result, err := cel2sql.ConvertParameterized(ast, +// cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) +// // result.SQL: "name = ? AND age > ?" +// db.Where(result.SQL, result.Parameters...) +func WithPlaceholderStyle(style PlaceholderStyle) ConvertOption { + return func(o *convertOptions) { + o.placeholderStyle = style + } +} + // Result represents the output of a CEL to SQL conversion with parameterized queries. // It contains the SQL string with placeholders ($1, $2, etc.) and the corresponding parameter values. type Result struct { @@ -362,17 +411,18 @@ func ConvertParameterized(ast *cel.Ast, opts ...ConvertOption) (*Result, error) paramStart = 1 } un := &converter{ - typeMap: checkedExpr.TypeMap, - schemas: options.schemas, - jsonVars: options.jsonVars, - columnAlias: options.columnAlias, - ctx: options.ctx, - logger: options.logger, - dialect: options.dialect, - maxDepth: options.maxDepth, - maxOutputLen: options.maxOutputLen, - parameterize: true, // Enable parameterization - paramCount: paramStart - 1, // First placeholder will be paramStart after first increment + typeMap: checkedExpr.TypeMap, + schemas: options.schemas, + jsonVars: options.jsonVars, + columnAlias: options.columnAlias, + ctx: options.ctx, + logger: options.logger, + dialect: options.dialect, + maxDepth: options.maxDepth, + maxOutputLen: options.maxOutputLen, + parameterize: true, // Enable parameterization + paramCount: paramStart - 1, // First placeholder will be paramStart after first increment + placeholderStyle: options.placeholderStyle, } if err := un.visit(checkedExpr.Expr); err != nil { @@ -381,6 +431,12 @@ func ConvertParameterized(ast *cel.Ast, opts ...ConvertOption) (*Result, error) } sql := un.str.String() + + if err := checkQuestionPlaceholders(options.placeholderStyle, sql, len(un.parameters)); err != nil { + options.logger.Error("parameterized conversion produced ambiguous placeholders", slog.Any("error", err)) + return nil, err + } + duration := time.Since(start) options.logger.LogAttrs(context.Background(), slog.LevelDebug, @@ -396,6 +452,37 @@ func ConvertParameterized(ast *cel.Ast, opts ...ConvertOption) (*Result, error) }, nil } +// checkQuestionPlaceholders verifies that every ? in the generated SQL is a bind +// placeholder. +// +// PostgreSQL's jsonb existence operator is itself a ?, so an expression like +// has(payload.field) against a JSONB column emits a ? that is an operator rather +// than a placeholder. Consumers of PlaceholderQuestion scan for ? without parsing +// SQL, so they would bind a value to that operator and shift every parameter +// after it — wrong rows, no error. Fail here instead. +// +// Only PostgreSQL is affected; every other dialect writes JSON existence as a +// function call. +func checkQuestionPlaceholders(style PlaceholderStyle, sql string, paramCount int) error { + if style != PlaceholderQuestion { + return nil + } + found := strings.Count(sql, "?") + if found == paramCount { + return nil + } + return &ConversionError{ + UserMessage: "cannot use PlaceholderQuestion with this expression: the generated SQL " + + "contains a ? that is an operator rather than a bind placeholder", + InternalDetails: fmt.Sprintf( + "generated SQL contains %d '?' but %d parameters; a dialect operator spelled '?' "+ + "(PostgreSQL jsonb existence) collides with question-mark placeholders. "+ + "Use PlaceholderDialect, or avoid has() on a JSONB column.", + found, paramCount), + WrappedErr: ErrUnsupportedDialectFeature, + } +} + type converter struct { str strings.Builder typeMap map[int64]*exprpb.Type @@ -405,6 +492,7 @@ type converter struct { ctx context.Context logger *slog.Logger dialect dialect.Dialect + placeholderStyle PlaceholderStyle depth int // Current recursion depth maxDepth int // Maximum allowed recursion depth maxOutputLen int // Maximum allowed SQL output length @@ -2296,6 +2384,18 @@ func (con *converter) visitTransformMapEntryComprehension(_ *exprpb.Expr, _ *Com return fmt.Errorf("%w: TRANSFORM_MAP_ENTRY comprehension requires map/JSON support (not yet implemented)", ErrInvalidComprehension) } +// writeParamPlaceholder emits one bind placeholder and advances the counter. +// Under PlaceholderQuestion the index is still tracked, so the placeholder count +// stays in step with len(parameters) even though it is not rendered. +func (con *converter) writeParamPlaceholder() { + con.paramCount++ + if con.placeholderStyle == PlaceholderQuestion { + con.str.WriteByte('?') + return + } + con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) +} + func (con *converter) visitConst(expr *exprpb.Expr) error { c := expr.GetConstExpr() switch c.ConstantKind.(type) { @@ -2311,8 +2411,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error { con.str.WriteString("NULL") case *exprpb.Constant_Int64Value: if con.parameterize { - con.paramCount++ - con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) + con.writeParamPlaceholder() con.parameters = append(con.parameters, c.GetInt64Value()) } else { i := strconv.FormatInt(c.GetInt64Value(), 10) @@ -2320,8 +2419,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error { } case *exprpb.Constant_Uint64Value: if con.parameterize { - con.paramCount++ - con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) + con.writeParamPlaceholder() con.parameters = append(con.parameters, c.GetUint64Value()) } else { ui := strconv.FormatUint(c.GetUint64Value(), 10) @@ -2329,8 +2427,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error { } case *exprpb.Constant_DoubleValue: if con.parameterize { - con.paramCount++ - con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) + con.writeParamPlaceholder() con.parameters = append(con.parameters, c.GetDoubleValue()) } else { d := strconv.FormatFloat(c.GetDoubleValue(), 'g', -1, 64) @@ -2344,8 +2441,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error { } if con.parameterize { - con.paramCount++ - con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) + con.writeParamPlaceholder() con.parameters = append(con.parameters, str) } else { con.dialect.WriteStringLiteral(&con.str, str) @@ -2354,8 +2450,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error { b := c.GetBytesValue() if con.parameterize { - con.paramCount++ - con.dialect.WriteParamPlaceholder(&con.str, con.paramCount) + con.writeParamPlaceholder() con.parameters = append(con.parameters, b) } else { // Validate byte array length to prevent resource exhaustion (CWE-400) diff --git a/docs/parameterized-queries.md b/docs/parameterized-queries.md index 14c1b3c..e916b41 100644 --- a/docs/parameterized-queries.md +++ b/docs/parameterized-queries.md @@ -8,6 +8,7 @@ This guide covers parameterized query support in cel2sql, including performance - [Why Use Parameterized Queries?](#why-use-parameterized-queries) - [API Reference](#api-reference) - [What Gets Parameterized?](#what-gets-parameterized) +- [Placeholder Style](#placeholder-style) - [Performance Optimization](#performance-optimization) - [Security Considerations](#security-considerations) - [Integration with database/sql](#integration-with-databasesql) @@ -117,6 +118,8 @@ All functional options from `Convert()` are supported: - `WithContext(ctx)` - Enable cancellation and timeouts - `WithLogger(logger)` - Enable structured logging - `WithMaxDepth(depth)` - Set recursion depth limit +- `WithParamStartIndex(n)` - First placeholder index, for splicing into a larger query you number yourself +- `WithPlaceholderStyle(style)` - Render `?` instead of the dialect's native placeholder; see [Placeholder Style](#placeholder-style) ### Result Type @@ -213,6 +216,92 @@ Parameters are numbered sequentially in the order they appear: // Parameters: [18, 100000.0, "John"] ``` +## Placeholder Style + +By default each dialect renders its native placeholder syntax: + +| Dialect | Placeholder | +|---------|-------------| +| PostgreSQL, DuckDB | `$1, $2` | +| MySQL, SQLite, Spark | `?, ?` | +| BigQuery | `@p1, @p2` | + +Some callers cannot accept pre-numbered placeholders. GORM, `sqlx.Rebind` and +squirrel each number placeholders from their own running count across the whole +statement, then rewrite them for the target driver. A fragment that arrives +already numbered is passed through untouched and reaches the database unbound. + +`WithPlaceholderStyle(PlaceholderQuestion)` emits `?` for every parameter and +leaves the numbering to them: + +```go +result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) +// result.SQL: "name = ? AND age > ?" +// result.Parameters: []any{"Alice", int64(30)} +``` + +The style is orthogonal to the dialect — it changes only the placeholder syntax, +never the SQL around it. For MySQL, SQLite and Spark, whose native placeholder is +already `?`, it produces identical output. + +### With GORM + +```go +result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) +if err != nil { + return err +} +db.Where(result.SQL, result.Parameters...).Find(&users) +``` + +GORM rewrites each `?` to `$1`, `$2` … for PostgreSQL, `@p1` for SQL Server, or +leaves it as `?` for MySQL and SQLite. + +### Why not rewrite `$n` to `?` yourself? + +Because not every `$1` in the output is a placeholder. A pattern passed to +`matches()` is inlined as a string literal, so this CEL: + +```cel +name.matches("a$1b") +``` + +produces SQL containing a literal `a$1b`. A `$\d+` search-and-replace would +corrupt the pattern and leave the parameter count wrong. Only the converter +knows which `$1` it wrote. + +### Interaction with WithParamStartIndex + +`WithParamStartIndex` has no visible effect under `PlaceholderQuestion` — there is +no index to offset. The two options serve opposite situations: `WithParamStartIndex` +for splicing into a query you number yourself, `PlaceholderQuestion` for handing the +numbering to a driver. + +### Limitation: PostgreSQL jsonb existence + +PostgreSQL spells the jsonb existence test as the `?` operator: + +```sql +metadata ? 'active' +``` + +That `?` is an operator, not a placeholder, and a consumer scanning for `?` cannot +tell the difference — it would bind a value to the operator and shift every +parameter after it. Rather than return SQL that silently binds wrong, +`ConvertParameterized` fails when `PlaceholderQuestion` would produce more `?` than +parameters: + +``` +cannot use PlaceholderQuestion with this expression: the generated SQL contains a ? +that is an operator rather than a bind placeholder +``` + +Use `PlaceholderDialect` for such expressions, or avoid `has()` on a JSONB column. +Only PostgreSQL is affected; every other dialect writes JSON existence as a +function call. + ## Performance Optimization ### Query Plan Caching diff --git a/placeholder_style_test.go b/placeholder_style_test.go new file mode 100644 index 0000000..998cf38 --- /dev/null +++ b/placeholder_style_test.go @@ -0,0 +1,196 @@ +package cel2sql_test + +import ( + "strings" + "testing" + + "cel.dev/cel-go/cel" + "cel.dev/cel-go/common/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/spandigital/cel2sql/v3" + "github.com/spandigital/cel2sql/v3/dialect" + "github.com/spandigital/cel2sql/v3/dialect/bigquery" + "github.com/spandigital/cel2sql/v3/dialect/duckdb" + "github.com/spandigital/cel2sql/v3/dialect/mysql" + "github.com/spandigital/cel2sql/v3/dialect/postgres" + "github.com/spandigital/cel2sql/v3/dialect/spark" + "github.com/spandigital/cel2sql/v3/dialect/sqlite" + "github.com/spandigital/cel2sql/v3/pg" + "github.com/spandigital/cel2sql/v3/schema" +) + +func placeholderTestEnv(t *testing.T) *cel.Env { + t.Helper() + env, err := cel.NewEnv( + cel.CustomTypeAdapter(types.DefaultTypeAdapter), + cel.Variable("name", cel.StringType), + cel.Variable("age", cel.IntType), + ) + require.NoError(t, err) + return env +} + +func placeholderTestAST(t *testing.T) *cel.Ast { + t.Helper() + ast, issues := placeholderTestEnv(t).Compile(`name == "Alice" && age > 30`) + require.NoError(t, issues.Err()) + return ast +} + +var placeholderDialects = []struct { + name dialect.Name + dialect dialect.Dialect + wantDefault string +}{ + {dialect.PostgreSQL, postgres.New(), `name = $1 AND age > $2`}, + {dialect.DuckDB, duckdb.New(), `name = $1 AND age > $2`}, + {dialect.BigQuery, bigquery.New(), `name = @p1 AND age > @p2`}, + {dialect.MySQL, mysql.New(), `name = ? AND age > ?`}, + {dialect.SQLite, sqlite.New(), `name = ? AND age > ?`}, + {dialect.Spark, spark.New(), `name = ? AND age > ?`}, +} + +// The default must stay byte-identical for every dialect — this is the +// regression guard for threading the style through visitConst. +func TestPlaceholderStyle_DefaultUnchanged(t *testing.T) { + ast := placeholderTestAST(t) + + for _, d := range placeholderDialects { + t.Run(string(d.name), func(t *testing.T) { + result, err := cel2sql.ConvertParameterized(ast, cel2sql.WithDialect(d.dialect)) + require.NoError(t, err) + + assert.Equal(t, d.wantDefault, result.SQL) + assert.Equal(t, []any{"Alice", int64(30)}, result.Parameters) + + explicit, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithDialect(d.dialect), + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderDialect)) + require.NoError(t, err) + assert.Equal(t, result.SQL, explicit.SQL, "PlaceholderDialect is the default") + }) + } +} + +func TestPlaceholderStyle_Question(t *testing.T) { + ast := placeholderTestAST(t) + + for _, d := range placeholderDialects { + t.Run(string(d.name), func(t *testing.T) { + result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithDialect(d.dialect), + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.NoError(t, err) + + assert.Equal(t, `name = ? AND age > ?`, result.SQL) + assert.Equal(t, []any{"Alice", int64(30)}, result.Parameters, + "parameters are unaffected by placeholder style") + assert.Equal(t, len(result.Parameters), strings.Count(result.SQL, "?")) + }) + } +} + +// MySQL, SQLite and Spark already emit ? natively, so the option is a no-op there. +func TestPlaceholderStyle_NoOpForQuestionMarkDialects(t *testing.T) { + ast := placeholderTestAST(t) + + for _, d := range []dialect.Dialect{mysql.New(), sqlite.New(), spark.New()} { + t.Run(string(d.Name()), func(t *testing.T) { + def, err := cel2sql.ConvertParameterized(ast, cel2sql.WithDialect(d)) + require.NoError(t, err) + + question, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithDialect(d), + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.NoError(t, err) + + assert.Equal(t, def.SQL, question.SQL) + assert.Equal(t, def.Parameters, question.Parameters) + }) + } +} + +// The two options address opposite situations, so combining them is not an error — +// there is simply no index left to offset. +func TestPlaceholderStyle_QuestionIgnoresParamStartIndex(t *testing.T) { + ast := placeholderTestAST(t) + + result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithParamStartIndex(5), + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.NoError(t, err) + + assert.Equal(t, `name = ? AND age > ?`, result.SQL) + assert.Equal(t, []any{"Alice", int64(30)}, result.Parameters) +} + +// This is why the option exists rather than leaving callers to rewrite $n to ?: +// matches() inlines its pattern as a string literal, so a $1 in the pattern is +// data, not a placeholder, and only the converter can tell them apart. +func TestPlaceholderStyle_RegexPatternKeepsLiteralDollar(t *testing.T) { + env, err := cel.NewEnv( + cel.CustomTypeAdapter(types.DefaultTypeAdapter), + cel.Variable("name", cel.StringType), + ) + require.NoError(t, err) + + ast, issues := env.Compile(`name.matches("a$1b") && name == "x"`) + require.NoError(t, issues.Err()) + + result, err := cel2sql.ConvertParameterized(ast, + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.NoError(t, err) + + assert.Contains(t, result.SQL, `a$1b`, "the regex pattern keeps its literal $1") + assert.Equal(t, 1, strings.Count(result.SQL, "?"), "only the bound literal is a placeholder") + assert.Equal(t, []any{"x"}, result.Parameters) +} + +func TestPlaceholderStyle_ConvertUnaffected(t *testing.T) { + ast := placeholderTestAST(t) + + def, err := cel2sql.Convert(ast) + require.NoError(t, err) + + styled, err := cel2sql.Convert(ast, cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.NoError(t, err) + + assert.Equal(t, def, styled, "Convert inlines literals and has no placeholders") + assert.NotContains(t, styled, "?") +} + +// PostgreSQL spells jsonb existence as the ? operator, which is indistinguishable +// from a placeholder to a consumer that scans for ?. Converting must fail rather +// than hand back SQL that would bind a value to an operator. +func TestPlaceholderStyle_QuestionRejectsPostgresJSONBExistence(t *testing.T) { + // The ? operator form is reached only for a direct field access on a column + // cel2sql recognises as JSON by name (isDirectJSONFieldAccess). + recordSchema := pg.NewSchema([]schema.FieldSchema{ + {Name: "metadata", Type: "jsonb", IsJSON: true, IsJSONB: true}, + }) + provider := pg.NewTypeProvider(map[string]pg.Schema{"record": recordSchema}) + + env, err := cel.NewEnv( + cel.CustomTypeProvider(provider), + cel.Variable("record", cel.ObjectType("record")), + ) + require.NoError(t, err) + + ast, issues := env.Compile(`has(record.metadata.active)`) + require.NoError(t, issues.Err()) + + schemas := provider.GetSchemas() + + dialectResult, err := cel2sql.ConvertParameterized(ast, cel2sql.WithSchemas(schemas)) + require.NoError(t, err, "the default style has no collision") + require.Contains(t, dialectResult.SQL, "?", "precondition: postgres emits the ? operator") + + _, err = cel2sql.ConvertParameterized(ast, + cel2sql.WithSchemas(schemas), + cel2sql.WithPlaceholderStyle(cel2sql.PlaceholderQuestion)) + require.Error(t, err) + assert.ErrorIs(t, err, cel2sql.ErrUnsupportedDialectFeature) + assert.Contains(t, err.Error(), "PlaceholderQuestion") +}