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") +}