Skip to content

Commit 53965fe

Browse files
fix: Use schema-based JSON field detection and correct table.column handling (#59, #20) (#60)
This commit fixes two critical issues with JSON path operator generation: **Issue #59: Incorrect JSON Path Operators** - Generated `obj->>'metadata'->>'user_name'` (applies ->> to table name) - Should be `obj.metadata->>'user_name'` (table.column then JSON operators) - Fixed by recognizing direct table.column access and only applying JSON operators to fields within the JSONB data **Issue #20: Hardcoded JSON Field Names** - Replaced hardcoded JSON field lists with schema-based type detection - Added `IsJSON` field to `pg.FieldSchema` to track JSON/JSONB columns - Created `ConvertWithSchemas()` function to pass schema information - Falls back to hardcoded lists for backward compatibility when schemas not provided **Changes:** - Added `IsJSON bool` field to `pg.FieldSchema` in `pg/provider.go` - Added `GetSchemas()` method to `TypeProvider` interface - Created `ConvertWithSchemas(ast, schemas)` function in `cel2sql.go` - Added `isFieldJSON()` and `getTableAndFieldFromSelectChain()` helper methods to converter - Updated `shouldUseJSONPath()` to use schema information first, then fall back to hardcoded list - Fixed `buildJSONPathInternal()` to properly handle direct table.column access without applying JSON operators to the table name - Updated all JSON-related tests to use `ConvertWithSchemas()` and set `IsJSON: true` - Updated test expectations to match correct PostgreSQL syntax **Testing:** - Added `json_operator_validation_test.go` - validates correct operator usage with real PostgreSQL - Added `json_integration_validation_test.go` - verifies generated SQL executes correctly - All existing tests pass with updated expectations - Validated against PostgreSQL 17 using testcontainers **Correct behavior:** - `obj.metadata.user_name` → `obj.metadata->>'user_name'` ✅ - `obj.metadata.settings.theme` → `obj.metadata->'settings'->>'theme'` ✅ - Uses `->` for intermediate JSON fields (keeps JSON type) - Uses `->>` for final JSON field (extracts as text) Fixes #59 Addresses #20 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent c265ffd commit 53965fe

File tree

7 files changed

+464
-75
lines changed

7 files changed

+464
-75
lines changed

cel2sql.go

Lines changed: 69 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,28 @@ import (
1111
"github.com/google/cel-go/common/operators"
1212
"github.com/google/cel-go/common/overloads"
1313
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
14+
15+
"github.com/spandigital/cel2sql/v2/pg"
1416
)
1517

1618
// Implementations based on `google/cel-go`'s unparser
1719
// https://github.com/google/cel-go/blob/master/parser/unparser.go
1820

1921
// Convert converts a CEL AST to a PostgreSQL SQL WHERE clause condition.
2022
func Convert(ast *cel.Ast) (string, error) {
23+
return ConvertWithSchemas(ast, nil)
24+
}
25+
26+
// ConvertWithSchemas converts a CEL AST to a PostgreSQL SQL WHERE clause condition,
27+
// using schema information to properly handle JSON/JSONB fields.
28+
func ConvertWithSchemas(ast *cel.Ast, schemas map[string]pg.Schema) (string, error) {
2129
checkedExpr, err := cel.AstToCheckedExpr(ast)
2230
if err != nil {
2331
return "", err
2432
}
2533
un := &converter{
2634
typeMap: checkedExpr.TypeMap,
35+
schemas: schemas,
2736
}
2837
if err := un.visit(checkedExpr.Expr); err != nil {
2938
return "", err
@@ -34,6 +43,7 @@ func Convert(ast *cel.Ast) (string, error) {
3443
type converter struct {
3544
str strings.Builder
3645
typeMap map[int64]*exprpb.Type
46+
schemas map[string]pg.Schema
3747
}
3848

3949
func (con *converter) visit(expr *exprpb.Expr) error {
@@ -57,6 +67,46 @@ func (con *converter) visit(expr *exprpb.Expr) error {
5767
return fmt.Errorf("unsupported expr: %v", expr)
5868
}
5969

70+
// isFieldJSON checks if a field in a table is a JSON/JSONB type using schema information
71+
func (con *converter) isFieldJSON(tableName, fieldName string) bool {
72+
if con.schemas == nil {
73+
return false
74+
}
75+
76+
schema, ok := con.schemas[tableName]
77+
if !ok {
78+
return false
79+
}
80+
81+
for _, field := range schema {
82+
if field.Name == fieldName {
83+
return field.IsJSON
84+
}
85+
}
86+
87+
return false
88+
}
89+
90+
// getTableAndFieldFromSelectChain extracts the table name and field name from a select expression chain
91+
// For obj.metadata, it returns ("obj", "metadata")
92+
func (con *converter) getTableAndFieldFromSelectChain(expr *exprpb.Expr) (string, string, bool) {
93+
selectExpr := expr.GetSelectExpr()
94+
if selectExpr == nil {
95+
return "", "", false
96+
}
97+
98+
fieldName := selectExpr.GetField()
99+
operand := selectExpr.GetOperand()
100+
101+
// Check if the operand is an identifier (table name)
102+
if identExpr := operand.GetIdentExpr(); identExpr != nil {
103+
tableName := identExpr.GetName()
104+
return tableName, fieldName, true
105+
}
106+
107+
return "", "", false
108+
}
109+
60110
func (con *converter) visitCall(expr *exprpb.Expr) error {
61111
c := expr.GetCallExpr()
62112
fun := c.GetFunction()
@@ -377,11 +427,11 @@ func (con *converter) callCasting(function string, _ *exprpb.Expr, args []*exprp
377427
func (con *converter) callMatches(target *exprpb.Expr, args []*exprpb.Expr) error {
378428
// CEL matches function: string.matches(pattern) or matches(string, pattern)
379429
// Convert to PostgreSQL: string ~ 'posix_pattern'
380-
430+
381431
// Get the string to match against
382432
var stringExpr *exprpb.Expr
383433
var patternExpr *exprpb.Expr
384-
434+
385435
if target != nil {
386436
// Method call: string.matches(pattern)
387437
stringExpr = target
@@ -393,18 +443,18 @@ func (con *converter) callMatches(target *exprpb.Expr, args []*exprpb.Expr) erro
393443
stringExpr = args[0]
394444
patternExpr = args[1]
395445
}
396-
446+
397447
if stringExpr == nil || patternExpr == nil {
398448
return errors.New("matches function requires both string and pattern arguments")
399449
}
400-
450+
401451
// Visit the string expression
402452
if err := con.visit(stringExpr); err != nil {
403453
return err
404454
}
405-
455+
406456
con.str.WriteString(" ~ ")
407-
457+
408458
// Visit the pattern expression and convert from RE2 to POSIX if it's a string literal
409459
if constExpr := patternExpr.GetConstExpr(); constExpr != nil && constExpr.GetStringValue() != "" {
410460
// Convert RE2 pattern to POSIX
@@ -427,7 +477,7 @@ func (con *converter) callMatches(target *exprpb.Expr, args []*exprpb.Expr) erro
427477
return err
428478
}
429479
}
430-
480+
431481
return nil
432482
}
433483

@@ -1442,45 +1492,45 @@ func isBinaryOrTernaryOperator(expr *exprpb.Expr) bool {
14421492
// Note: This is a basic conversion for common patterns. Full RE2 to POSIX conversion is complex.
14431493
func convertRE2ToPOSIX(re2Pattern string) string {
14441494
posixPattern := re2Pattern
1445-
1495+
14461496
// Basic conversions for common differences between RE2 and POSIX:
1447-
1497+
14481498
// 1. Word boundaries: \b -> [[:<:]] and [[:<:]] (PostgreSQL extension)
14491499
// Note: PostgreSQL supports \y for word boundaries in some contexts
14501500
posixPattern = strings.ReplaceAll(posixPattern, `\b`, `\y`)
1451-
1501+
14521502
// 2. Non-word boundaries: \B -> [^[:alnum:]_] (approximate)
14531503
// This is a simplification; exact conversion is complex
14541504
posixPattern = strings.ReplaceAll(posixPattern, `\B`, `[^[:alnum:]_]`)
1455-
1505+
14561506
// 3. Digit shortcuts: \d -> [[:digit:]] or [0-9]
14571507
posixPattern = strings.ReplaceAll(posixPattern, `\d`, `[[:digit:]]`)
1458-
1508+
14591509
// 4. Non-digit shortcuts: \D -> [^[:digit:]] or [^0-9]
14601510
posixPattern = strings.ReplaceAll(posixPattern, `\D`, `[^[:digit:]]`)
1461-
1511+
14621512
// 5. Word character shortcuts: \w -> [[:alnum:]_]
14631513
posixPattern = strings.ReplaceAll(posixPattern, `\w`, `[[:alnum:]_]`)
1464-
1514+
14651515
// 6. Non-word character shortcuts: \W -> [^[:alnum:]_]
14661516
posixPattern = strings.ReplaceAll(posixPattern, `\W`, `[^[:alnum:]_]`)
1467-
1517+
14681518
// 7. Whitespace shortcuts: \s -> [[:space:]]
14691519
posixPattern = strings.ReplaceAll(posixPattern, `\s`, `[[:space:]]`)
1470-
1520+
14711521
// 8. Non-whitespace shortcuts: \S -> [^[:space:]]
14721522
posixPattern = strings.ReplaceAll(posixPattern, `\S`, `[^[:space:]]`)
1473-
1523+
14741524
// Note: Many RE2 features are not directly convertible to POSIX ERE:
14751525
// - Lookahead/lookbehind assertions (?=...), (?!...), (?<=...), (?<!...)
14761526
// - Non-capturing groups (?:...)
14771527
// - Named groups (?P<name>...)
14781528
// - Case-insensitive flags (?i)
14791529
// - Multiline flags (?m)
14801530
// - Unicode character classes
1481-
//
1531+
//
14821532
// For these cases, the pattern is returned as-is, which may cause PostgreSQL errors
14831533
// if the pattern uses unsupported RE2 features.
1484-
1534+
14851535
return posixPattern
14861536
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.24
55
require (
66
github.com/google/cel-go v0.26.0
77
github.com/jackc/pgx/v5 v5.7.5
8+
github.com/lib/pq v1.10.9
89
github.com/spandigital/cel2sql v0.0.0-20250719023919-4359e01a942f
910
github.com/stretchr/testify v1.10.0
1011
github.com/testcontainers/testcontainers-go v0.38.0

0 commit comments

Comments
 (0)