Skip to content

Conversation

@richardwooding
Copy link
Contributor

Summary

Fixes two critical issues with JSON path operator generation:

Issue #59: Incorrect operator usage - was applying ->> to table names
Issue #20: Hardcoded JSON field names causing maintainability and security issues

Problem

Issue #59: Wrong SQL Generated

-- CEL: obj.metadata.user_name == "test"
-- Generated (WRONG): obj->>'metadata'->>'user_name' = 'test'
-- Should be:         obj.metadata->>'user_name' = 'test'

The code was treating obj (the table name) as something that needs JSON operators, when it should recognize obj.metadata as a regular table.column reference.

Issue #20: Hardcoded Field Names

JSON field detection relied on hardcoded lists in 7+ locations:

  • Inconsistent across functions
  • No way for users to specify their own JSON columns
  • Security risk if fields missed
  • Maintenance burden

Solution

1. Schema-Based Type Detection

  • Added IsJSON bool field to pg.FieldSchema
  • Created ConvertWithSchemas(ast, schemas) function
  • Converter now checks schema information to determine if a field is JSON/JSONB
  • Falls back to hardcoded lists for backward compatibility

2. Fixed table.column Handling

  • Added getTableAndFieldFromSelectChain() to identify direct table.column access
  • Updated buildJSONPathInternal() to render table.column correctly before applying JSON operators
  • Now properly distinguishes between table references and JSON paths

Changes

Core Changes

  • cel2sql.go: Added ConvertWithSchemas(), schema lookup methods, and schemas field to converter
  • json.go: Fixed shouldUseJSONPath() and buildJSONPathInternal() to use schema information
  • pg/provider.go: Added IsJSON field to FieldSchema, GetSchemas() method to interface

Testing

  • json_operator_validation_test.go: Validates correct operator usage with real PostgreSQL
  • json_integration_validation_test.go: Verifies generated SQL executes correctly
  • Updated all JSON tests to use ConvertWithSchemas() and set IsJSON: true

PostgreSQL Validation

Created integration tests using testcontainers with PostgreSQL 17:

Correct SQL that works:

obj.metadata->>'user_name' = 'test'
obj.metadata->'settings'->>'theme' = 'dark'

Wrong SQL that fails:

obj->>'metadata'->>'user_name' = 'test'  
-- ERROR: operator does not exist: obj ->> unknown

Backward Compatibility

  • Convert(ast) still works without schema information
  • Falls back to hardcoded field lists when schemas not provided
  • All existing tests pass with minimal changes

Test Results

All tests pass:

make test
# All tests: PASS
make lint  
# 0 issues

Closes

Closes #59
Addresses #20

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

…andling (#59, #20)

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate JSON Path Operator Usage: -> vs ->> for Intermediate Fields

1 participant