Skip to content

Commit 730fb45

Browse files
richardwoodingclaude
richardwooding
and
claude
authored
fix: Complete schema-based JSON detection and remove hardcoded field lists (#61, #59) (#62)
* fix: Remove hardcoded JSON field lists in favor of schema-based detection (#61) This commit removes all hardcoded field name lists from JSON detection functions and makes them rely entirely on schema information provided via ConvertWithSchemas(). Changes: - Extended pg.FieldSchema with IsJSONB and ElementType fields - Updated LoadTableSchema() to populate these fields from database - Added schema lookup methods: isFieldJSONB(), isFieldArray(), getFieldElementType() - Removed hardcoded field lists from: - shouldUseJSONPath() - removed jsonFields list - hasJSONFieldInChain() - now uses schema-based detection - isJSONArrayField() - removed jsonArrayFields and arrayFieldNames maps - isJSONBField() - removed jsonbFields and jsonbParentFields lists - getJSONArrayFunction() - removed simpleArrayFields and complexArrayFields lists - Updated all tests to use ConvertWithSchemas() - Disabled test cases requiring knowledge of internal JSON structure (PostgreSQL's information_schema doesn't provide this) Backward compatibility: Users must now provide schema information via ConvertWithSchemas() for JSON field detection to work properly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Use functional options pattern for Convert API Replace ConvertWithSchemas() with a more flexible API using functional options. The Convert() function now accepts variadic options, making it more extensible for future enhancements. Changes: - Add ConvertOption type and WithSchemas() option function - Refactor Convert() to accept variadic options - Remove ConvertWithSchemas() function - Update all test files to use new API: Convert(ast, cel2sql.WithSchemas(schemas)) - Update examples/load_table_schema to demonstrate new API This API design follows Go best practices and allows for easier addition of new options in the future without breaking changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 53965fe commit 730fb45

File tree

11 files changed

+423
-356
lines changed

11 files changed

+423
-356
lines changed

‎cel2sql.go‎

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,50 @@ import (
1818
// Implementations based on `google/cel-go`'s unparser
1919
// https://github.com/google/cel-go/blob/master/parser/unparser.go
2020

21-
// Convert converts a CEL AST to a PostgreSQL SQL WHERE clause condition.
22-
func Convert(ast *cel.Ast) (string, error) {
23-
return ConvertWithSchemas(ast, nil)
21+
// ConvertOption is a functional option for configuring the Convert function.
22+
type ConvertOption func(*convertOptions)
23+
24+
// convertOptions holds configuration options for the Convert function.
25+
type convertOptions struct {
26+
schemas map[string]pg.Schema
27+
}
28+
29+
// WithSchemas provides schema information for proper JSON/JSONB field handling.
30+
// This option is required for correct SQL generation when using JSON/JSONB fields.
31+
//
32+
// Example:
33+
//
34+
// schemas := provider.GetSchemas()
35+
// sql, err := cel2sql.Convert(ast, cel2sql.WithSchemas(schemas))
36+
func WithSchemas(schemas map[string]pg.Schema) ConvertOption {
37+
return func(o *convertOptions) {
38+
o.schemas = schemas
39+
}
2440
}
2541

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) {
42+
// Convert converts a CEL AST to a PostgreSQL SQL WHERE clause condition.
43+
// Options can be provided to configure the conversion behavior.
44+
//
45+
// Example without options:
46+
//
47+
// sql, err := cel2sql.Convert(ast)
48+
//
49+
// Example with schema information for JSON/JSONB support:
50+
//
51+
// sql, err := cel2sql.Convert(ast, cel2sql.WithSchemas(schemas))
52+
func Convert(ast *cel.Ast, opts ...ConvertOption) (string, error) {
53+
options := &convertOptions{}
54+
for _, opt := range opts {
55+
opt(options)
56+
}
57+
2958
checkedExpr, err := cel.AstToCheckedExpr(ast)
3059
if err != nil {
3160
return "", err
3261
}
3362
un := &converter{
3463
typeMap: checkedExpr.TypeMap,
35-
schemas: schemas,
64+
schemas: options.schemas,
3665
}
3766
if err := un.visit(checkedExpr.Expr); err != nil {
3867
return "", err
@@ -107,6 +136,66 @@ func (con *converter) getTableAndFieldFromSelectChain(expr *exprpb.Expr) (string
107136
return "", "", false
108137
}
109138

139+
// isFieldJSONB checks if a field in a table is specifically JSONB (vs JSON) using schema information
140+
func (con *converter) isFieldJSONB(tableName, fieldName string) bool {
141+
if con.schemas == nil {
142+
return false
143+
}
144+
145+
schema, ok := con.schemas[tableName]
146+
if !ok {
147+
return false
148+
}
149+
150+
for _, field := range schema {
151+
if field.Name == fieldName {
152+
return field.IsJSONB
153+
}
154+
}
155+
156+
return false
157+
}
158+
159+
// isFieldArray checks if a field in a table is an array using schema information
160+
func (con *converter) isFieldArray(tableName, fieldName string) bool {
161+
if con.schemas == nil {
162+
return false
163+
}
164+
165+
schema, ok := con.schemas[tableName]
166+
if !ok {
167+
return false
168+
}
169+
170+
for _, field := range schema {
171+
if field.Name == fieldName {
172+
return field.Repeated
173+
}
174+
}
175+
176+
return false
177+
}
178+
179+
// getFieldElementType returns the element type of an array field using schema information
180+
func (con *converter) getFieldElementType(tableName, fieldName string) string {
181+
if con.schemas == nil {
182+
return ""
183+
}
184+
185+
schema, ok := con.schemas[tableName]
186+
if !ok {
187+
return ""
188+
}
189+
190+
for _, field := range schema {
191+
if field.Name == fieldName && field.Repeated {
192+
return field.ElementType
193+
}
194+
}
195+
196+
return ""
197+
}
198+
110199
func (con *converter) visitCall(expr *exprpb.Expr) error {
111200
c := expr.GetCallExpr()
112201
fun := c.GetFunction()

‎examples/load_table_schema/main.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ func exampleWithPredefinedSchema() {
6565
continue
6666
}
6767

68-
sqlCondition, err := cel2sql.Convert(ast)
68+
// Convert to SQL with schema information for JSON field detection
69+
sqlCondition, err := cel2sql.Convert(ast, cel2sql.WithSchemas(provider.GetSchemas()))
6970
if err != nil {
7071
log.Printf("Error converting %s: %v", expr, err)
7172
continue

‎field_validation_test.go‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ func TestFieldNameValidation_Integration(t *testing.T) {
2424
})
2525

2626
tests := []struct {
27-
name string
28-
celExpr string
29-
expectError bool
27+
name string
28+
celExpr string
29+
expectError bool
3030
errorContains string
3131
}{
3232
// Valid field names should work
@@ -43,33 +43,33 @@ func TestFieldNameValidation_Integration(t *testing.T) {
4343

4444
// Reserved keywords should be rejected
4545
{
46-
name: "reserved keyword: select",
47-
celExpr: `obj.select == "test"`,
48-
expectError: true,
46+
name: "reserved keyword: select",
47+
celExpr: `obj.select == "test"`,
48+
expectError: true,
4949
errorContains: "reserved SQL keyword",
5050
},
5151
{
52-
name: "reserved keyword: where",
53-
celExpr: `obj.where == "test"`,
54-
expectError: true,
52+
name: "reserved keyword: where",
53+
celExpr: `obj.where == "test"`,
54+
expectError: true,
5555
errorContains: "reserved SQL keyword",
5656
},
5757
{
58-
name: "reserved keyword: from",
59-
celExpr: `obj.from == "test"`,
60-
expectError: true,
58+
name: "reserved keyword: from",
59+
celExpr: `obj.from == "test"`,
60+
expectError: true,
6161
errorContains: "reserved SQL keyword",
6262
},
6363
{
64-
name: "reserved keyword: union",
65-
celExpr: `obj.union == "test"`,
66-
expectError: true,
64+
name: "reserved keyword: union",
65+
celExpr: `obj.union == "test"`,
66+
expectError: true,
6767
errorContains: "reserved SQL keyword",
6868
},
6969
{
70-
name: "reserved keyword: drop",
71-
celExpr: `obj.drop == "test"`,
72-
expectError: true,
70+
name: "reserved keyword: drop",
71+
celExpr: `obj.drop == "test"`,
72+
expectError: true,
7373
errorContains: "reserved SQL keyword",
7474
},
7575
}
@@ -111,10 +111,10 @@ func TestFieldNameValidation_Integration(t *testing.T) {
111111
// TestFieldNameValidation_Identifiers tests identifier validation
112112
func TestFieldNameValidation_Identifiers(t *testing.T) {
113113
tests := []struct {
114-
name string
115-
varName string
116-
celExpr string
117-
expectError bool
114+
name string
115+
varName string
116+
celExpr string
117+
expectError bool
118118
errorContains string
119119
}{
120120
{
@@ -130,17 +130,17 @@ func TestFieldNameValidation_Identifiers(t *testing.T) {
130130
expectError: false,
131131
},
132132
{
133-
name: "reserved keyword identifier",
134-
varName: "select",
135-
celExpr: `select == "test"`,
136-
expectError: true,
133+
name: "reserved keyword identifier",
134+
varName: "select",
135+
celExpr: `select == "test"`,
136+
expectError: true,
137137
errorContains: "reserved SQL keyword",
138138
},
139139
{
140-
name: "reserved keyword: table",
141-
varName: "table",
142-
celExpr: `table == 5`,
143-
expectError: true,
140+
name: "reserved keyword: table",
141+
varName: "table",
142+
celExpr: `table == 5`,
143+
expectError: true,
144144
errorContains: "reserved SQL keyword",
145145
},
146146
}
@@ -218,9 +218,9 @@ func TestFieldNameValidation_PreventsSQLInjection(t *testing.T) {
218218
// Comprehensive validation testing is in utils_test.go
219219

220220
maliciousPatterns := []struct {
221-
name string
222-
celExpr string
223-
reason string
221+
name string
222+
celExpr string
223+
reason string
224224
}{
225225
{
226226
name: "cannot use semicolon in field name",
@@ -254,9 +254,9 @@ func TestFieldNameValidation_PreventsSQLInjection(t *testing.T) {
254254
// TestFieldNameValidation_EdgeCases tests edge cases
255255
func TestFieldNameValidation_EdgeCases(t *testing.T) {
256256
tests := []struct {
257-
name string
258-
fieldName string
259-
shouldPass bool
257+
name string
258+
fieldName string
259+
shouldPass bool
260260
}{
261261
{
262262
name: "single character",

0 commit comments

Comments
 (0)