feat: backport features and fix from observeinc/cel2sql fork

[richardwooding]

Summary

Backports four commits from the observeinc/cel2sql fork — three additive options plus one bug fix. Skips the fork's module-path rename commit. All commits attribute the original author via Co-Authored-By.

• WithJSONVariables(vars ...string) — declare flat JSONB variables; tags.color (or tags["color"]) emits tags->>'color'. Previously cel2sql only recognized JSONB through the two-level table.jsonColumn.key pattern.
• WithColumnAliases(map[string]string) — map CEL variable names to different SQL columns (e.g., name → usr_name). Aliases are validated through the dialect's field-name validator. Combines with WithJSONVariables.
• WithParamStartIndex(int) — first placeholder index for ConvertParameterized (e.g., $5, $6, ...). Useful when embedding the CEL fragment inside a larger parameterized query. Values < 1 clamp to 1.
• BREAKING fix: removed the name-based numeric-cast heuristic from visitIdent. Identifiers named score/value/num/amount/count/level are no longer auto-cast to ::numeric — the heuristic was a footgun that misfired on plain (non-JSON) variables sharing those names. JSON text-extraction comparisons in visitCall are unaffected.

Notes

• Dropped the fork's unused markJSONIterVar/unmarkJSONIterVar helpers per the project's "no scaffolding for hypothetical futures" guidance — the empty jsonIterVars map already produces the intended "heuristic disabled" behavior.
• Added 5 tests for WithParamStartIndex (the fork's PR shipped without any).
• All Co-Authored-By: trailers attribute david.lotfi <david.lotfi@observeinc.com>.

Test plan

• golangci-lint run — 0 issues
• go test -short -race ./... — all unit tests pass (15 new, all existing). TestPostgreSQL17Compatibility fails locally due to a missing Docker daemon; this is pre-existing on main and unrelated.
• CI green
• make ci on a Docker-enabled host (full integration suite)

Commits

• feat: add WithJSONVariables option for flat JSONB variable support
• feat: add WithColumnAliases option for CEL-to-SQL column name mapping
• fix: remove name-based numeric casting heuristic from visitIdent
• feat: add WithParamStartIndex for embedding CEL SQL in larger queries
• docs: document backported observe options + breaking change

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Backports several features from the observeinc/cel2sql fork to extend CEL→SQL conversion configurability (flat JSONB variables, column aliasing, configurable parameter start index) and removes a brittle numeric-casting heuristic.

Changes:

• Added WithJSONVariables, WithColumnAliases, and WithParamStartIndex options to the converter.
• Updated JSON-path detection / numeric-cast heuristic behavior and added new unit tests for the new options.
• Documented the new options and the breaking numeric-cast heuristic removal in project docs/changelog.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
cel2sql.go	Adds new conversion options (JSON variables, column aliases, param start index) and wires them into conversion and identifier rendering.
json.go	Extends JSON-path detection to recognize flat JSON variables and removes name-based numeric-cast heuristics.
json_variables_test.go	New tests covering WithJSONVariables and WithColumnAliases.
param_start_index_test.go	New tests covering WithParamStartIndex.
README.md	Documents the newly added options.
CLAUDE.md	Adds extended documentation/examples for the new options.
CHANGELOG.md	Records the new options and the breaking heuristic change.

Comments suppressed due to low confidence (1)

json.go:51

• WithJSONVariables currently only triggers JSON-path generation when the immediate operand is an IdentExpr. For nested access like tags.corpus.section (where the operand is itself a SelectExpr), shouldUseJSONPath returns false, and the converter falls back to regular select rendering, producing invalid SQL such as tags->>'corpus'.section. Consider (1) treating any select chain rooted at a JSON variable as JSON-path eligible (e.g., extend shouldUseJSONPath/hasJSONFieldInChain to detect con.isJSONVariable at the root), and (2) updating buildJSONPathInternal to avoid the table.column shortcut when tableName is a JSON variable, so nested keys render as tags->'corpus'->>'section'. Adding a test for nested keys would prevent regressions.

// shouldUseJSONPath determines if we should use JSON path operators for field access.
// This function checks if the operand represents a JSON/JSONB field using schema
// information or the WithJSONVariables option for flat JSONB variables.
func (con *converter) shouldUseJSONPath(operand *exprpb.Expr, _ string) bool {
	// Check if the operand is a flat variable declared as JSONB via WithJSONVariables
	if identExpr := operand.GetIdentExpr(); identExpr != nil {
		if con.isJSONVariable(identExpr.GetName()) {
			return true
		}
	}

	// Check if the operand is a direct table.column access where column is JSON
	if selectExpr := operand.GetSelectExpr(); selectExpr != nil {
		// For obj.metadata, check if metadata is a JSON column in obj table
		if tableName, fieldName, ok := con.getTableAndFieldFromSelectChain(operand); ok {
			// Use schema information to determine if this field is JSON
			isJSON := con.isFieldJSON(tableName, fieldName)
			con.logger.LogAttrs(context.Background(), slog.LevelDebug,
				"JSON path detection",
				slog.String("table", tableName),
				slog.String("field", fieldName),
				slog.Bool("is_json", isJSON),
			)
			return isJSON
		}

		// Check if there's a JSON field somewhere in the operand chain
		if con.hasJSONFieldInChain(operand) {
			con.logger.Debug("JSON field detected in select chain")
			return true
		}
	}

[richardwooding]

Fixed in 16cc2ea. The cel2sql.go:416 dead-code observation was correct — jsonIterVars was never populated. Removed the field, the needsNumericCasting function, and the unreachable cast branch in visitIdent. The heuristic-disabled behavior is preserved by simply omitting the cast logic; the breaking-change CHANGELOG entry still applies.

[richardwooding]

Fixed in 16cc2ea. Both table-driven tests now always assert result.Parameters: when wantArgs is nil they use assert.Empty(...); otherwise they use assert.Equal(wantArgs, ...). Catches regressions where parameters might be unexpectedly collected for boolean / no-arg cases.

[richardwooding]

Fixed in 16cc2ea — same pattern: assert.Empty when wantArgs == nil, assert.Equal otherwise. Applied via replace_all so both TestWithJSONVariables_* and TestWithColumnAliases table tests now assert empty-param cases explicitly.

[richardwooding]

Good catch — fixed in 16cc2ea. Replaced the pre-write + no-op-writeBase pattern with WriteJSONFieldAccess(&con.str, func() error { return con.visit(m) }, fieldName, true). PostgreSQL/DuckDB output is unchanged (operator-style); SQLite (json_extract(base, $.field)) and BigQuery (JSON_VALUE(base, $.field)) now correctly wrap the base inside the function call.

[richardwooding]

Pushed 16cc2ea addressing all four Copilot inline comments + the suppressed low-confidence comment about nested JSON-variable access.

Resolved:

1. ✅ visitCallMapIndex now uses the writeBase callback correctly (works for SQLite json_extract and BigQuery JSON_VALUE).
2. ✅ Dead jsonIterVars field and needsNumericCasting function removed; visitIdent simplified.
3. ✅ Tests now always assert result.Parameters (assert.Empty when none expected, assert.Equal otherwise).
4. ✅ Nested JSON-variable dot-access fixed: tags.corpus.section → tags->'corpus'->>'section' (was producing invalid tags->>'corpus'.section). New TestWithJSONVariables_NestedDotNotation covers 2- and 3-level depth.

Status checks:

• Go Vulnerability Check and OSV Security Scan failures are pre-existing on main (transitive Docker SDK vuln via testcontainers-go in test/example code, unrelated to this PR — Security workflow has been red since at least 2026-04-12). Out of scope for the backport; should be tracked separately.
• All other checks green: lint, unit tests, fuzz, benchmarks, gosec.