From a333e897a91b2097d7049aadd924cc73f35be793 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Wed, 22 Oct 2025 11:42:06 +0530 Subject: [PATCH 1/5] WIP --- JSONB_FILTER_ARCHITECTURE.md | 219 ++++++++++ NESTED_FILTER_ANALYSIS.md | 382 ++++++++++++++++++ .../documentstore/DocStoreQueryV1Test.java | 233 +++++++++++ .../flat_json_contains_filter_response.json | 12 + .../query/flat_json_eq_filter_response.json | 9 + .../query/flat_json_gt_filter_response.json | 22 + .../query/flat_json_in_filter_response.json | 14 + .../query/flat_json_lt_filter_response.json | 22 + .../query/flat_json_neq_filter_response.json | 14 + ...lat_json_not_contains_filter_response.json | 37 ++ .../impl/JsonIdentifierExpression.java | 13 + .../parser/SelectTypeExpressionVisitor.java | 5 + ...gresNotContainsRelationalFilterParser.java | 26 +- .../PostgresNotInRelationalFilterParser.java | 20 +- ...gresRelationalFilterParserFactoryImpl.java | 36 +- .../PostgresSelectionQueryTransformer.java | 11 + .../PostgresUnnestQueryTransformer.java | 6 + ...taAccessorIdentifierExpressionVisitor.java | 12 + ...tgresFieldIdentifierExpressionVisitor.java | 9 + .../PostgresIdentifierExpressionVisitor.java | 6 + .../impl/JsonIdentifierExpressionTest.java | 35 ++ settings-gradle.lockfile | 4 + 22 files changed, 1129 insertions(+), 18 deletions(-) create mode 100644 JSONB_FILTER_ARCHITECTURE.md create mode 100644 NESTED_FILTER_ANALYSIS.md create mode 100644 document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json create mode 100644 document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json create mode 100644 settings-gradle.lockfile diff --git a/JSONB_FILTER_ARCHITECTURE.md b/JSONB_FILTER_ARCHITECTURE.md new file mode 100644 index 00000000..ef3de3ac --- /dev/null +++ b/JSONB_FILTER_ARCHITECTURE.md @@ -0,0 +1,219 @@ +# JSONB Filter Architecture - Simple instanceof Design + +## Overview + +This document describes the architecture used to handle CONTAINS and IN filters on JSONB nested columns using simple, direct `instanceof` type checking. + +## Architecture Components + +### 1. **Type Detection via instanceof** + +```java +// Simple, direct type check +boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; +``` + +**Purpose:** Determines if an expression represents a JSON field access (JSONB column). + +**Benefits:** +- ✅ **Simple & Direct** - Immediately clear what's being checked +- ✅ **Standard Java** - Everyone understands instanceof +- ✅ **No extra methods** - Doesn't pollute interfaces +- ✅ **Type-safe** - Compile-time type checking +- ✅ **Appropriate** - Right tool for type checking + +### 2. **Factory Pattern for Parser Selection** + +#### `PostgresRelationalFilterParserFactoryImpl` +```java +// Location: postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java + +public PostgresRelationalFilterParser parser( + RelationalExpression expression, + PostgresQueryParser postgresQueryParser) { + + // Determine parser type based on collection and field type + boolean useJsonParser = shouldUseJsonParser(expression, postgresQueryParser); + + if (expression.getOperator() == CONTAINS) { + return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; + } else if (expression.getOperator() == IN) { + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; + } + // ... +} + +private boolean shouldUseJsonParser( + RelationalExpression expression, + PostgresQueryParser postgresQueryParser) { + + boolean isFlatCollection = + postgresQueryParser.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + // Simple instanceof check + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + + return !isFlatCollection || isJsonField; +} +``` + +**Decision Logic:** +``` +Use JSON Parser when: +1. Nested collections (entire document is JSONB), OR +2. JSON fields within flat collections (JSONB columns) + +Use Standard Parser when: +- Flat collections with first-class columns (e.g., text[], int) +``` + +### 3. **Parser Implementations** + +#### For JSON Fields (JSONB columns) +- **`PostgresContainsRelationalFilterParser`** - Uses `@>` with JSONB casting +- **`PostgresInRelationalFilterParser`** - Uses `jsonb_typeof()` and `@>` operators + +```sql +-- CONTAINS on JSON field +props @> '["value"]'::jsonb + +-- IN on JSON field +(jsonb_typeof(to_jsonb(props)) = 'array' AND to_jsonb(props) @> jsonb_build_array(?)) +``` + +#### For Non-JSON Fields (Regular PostgreSQL arrays) +- **`PostgresContainsRelationalFilterParserNonJsonField`** - Uses array `@>` operator +- **`PostgresInRelationalFilterParserNonJsonField`** - Uses array `&&` operator + +```sql +-- CONTAINS on array column +tags_unnested @> ARRAY[?]::text[] + +-- IN on array column +ARRAY[tags_unnested]::text[] && ARRAY[?, ?]::text[] +``` + +## Design Principles + +### ✅ **1. Pragmatic Simplicity** +```java +// Direct type checking - simple and clear +boolean isJsonField = expression instanceof JsonIdentifierExpression; +``` + +**Why instanceof is appropriate here:** +- **Clarity** - Intent is immediately obvious +- **Standard** - Well-known Java idiom +- **Appropriate** - This is exactly what instanceof is for +- **KISS** - Keep It Simple, Stupid +- **YAGNI** - You Aren't Gonna Need It (no extra abstraction) + +### ✅ **2. Single Responsibility** +- **Expression classes** - Represent field access patterns +- **Factory** - Selects appropriate parser based on field type +- **Parsers** - Generate SQL for specific operators and field types +- Each class does one thing well + +### ✅ **3. Type Safety** +- `instanceof` is compile-time type-safe +- Type hierarchy provides structure +- No runtime casting errors in normal flow + +### ✅ **4. Easy to Extend** +New expression types can be added by: +1. Create new expression class extending `IdentifierExpression` +2. Factory automatically uses correct parser based on `instanceof` check +3. Add new `instanceof` check only if special handling needed + +## Extension Points + +### Adding New Operators + +1. **Create parser interface:** +```java +interface PostgresMyOperatorParserInterface extends PostgresRelationalFilterParser { + String parse(RelationalExpression expression, PostgresRelationalFilterContext context); +} +``` + +2. **Create JSON and non-JSON implementations:** +```java +class PostgresMyOperatorParser implements PostgresMyOperatorParserInterface { } +class PostgresMyOperatorParserNonJsonField implements PostgresMyOperatorParserInterface { } +``` + +3. **Update factory:** +```java +if (expression.getOperator() == MY_OPERATOR) { + return useJsonParser ? jsonFieldMyOperatorParser : nonJsonFieldMyOperatorParser; +} +``` + +### Adding New Expression Types + +1. **Create the new expression class:** +```java +public class MyNewExpression extends IdentifierExpression { + // ... your implementation +} +``` + +2. **Add instanceof check in factory if special handling needed:** +```java +if (expression.getLhs() instanceof MyNewExpression) { + // Use special parser for this type +} +``` + +That's it! No method overriding needed unless special behavior is required. + +## Testing Strategy + +### Unit Tests +- Factory selects correct parser based on field type +- Each parser generates correct SQL +- Type detection works correctly for all expression types + +### Integration Tests +- CONTAINS filters work on JSONB columns +- IN filters work on JSONB columns +- Mixed queries with JSON and non-JSON fields +- Nested vs flat collection behavior + +## Summary + +**The architecture uses simple instanceof checking:** +1. ✅ Direct `instanceof JsonIdentifierExpression` type checks +2. ✅ Factory pattern for parser selection based on field type +3. ✅ Clean separation between JSON and non-JSON parsers +4. ✅ No unnecessary abstraction layers + +**Key Benefits:** +- ✅ **Simple** - Immediately understandable +- ✅ **Direct** - No indirection or extra methods +- ✅ **Standard Java** - Uses well-known idioms +- ✅ **Type-Safe** - Compile-time checking +- ✅ **Appropriate** - Right tool for the job +- ✅ **No pollution** - Doesn't add methods to wrong interfaces + +**Why instanceof is Best Here:** +- ✅ Type checking is exactly what `instanceof` is for +- ✅ Only 3 places in code that need the check +- ✅ Not a "type property" - just implementation detail +- ✅ Simpler than any abstraction (KISS principle) +- ✅ Easier to understand and maintain + +**Comparison with Alternatives:** + +| Approach | Pros | Cons | Verdict | +|----------|------|------|---------| +| **instanceof** ✅ | Simple, direct, clear | Some say "not OOP" | **Best for this case** | +| Polymorphic Method | "More OOP" | Pollutes interfaces, over-engineered | Overkill | +| Visitor Pattern | Very extensible | Way too complex for simple check | Overkill | +| Marker Interface | Type-safe | Still uses instanceof | Adds complexity | + +**When to use each pattern:** +- **instanceof** ✅ - Simple type checks (this case) +- **Polymorphic Method** - Behavior that's core to type contract +- **Visitor Pattern** - Multiple complex operations across types +- **Marker Interface** - Semantic grouping of multiple types diff --git a/NESTED_FILTER_ANALYSIS.md b/NESTED_FILTER_ANALYSIS.md new file mode 100644 index 00000000..6dfd48b1 --- /dev/null +++ b/NESTED_FILTER_ANALYSIS.md @@ -0,0 +1,382 @@ +# Analysis: Nested JSON Field Filters for Flat PostgreSQL Collections + +## Current State + +### Nested Collections (Working) +For nested PostgreSQL collections where the entire document is stored in a JSONB column (`document`), filters on nested fields like `props.colors` work: + +```java +// Test case: testContains (line 1527-1547) +Query query = Query.builder() + .addSelection(IdentifierExpression.of("props.colors")) + .setFilter( + Filter.builder() + .expression( + RelationalExpression.of( + IdentifierExpression.of("props.colors"), + CONTAINS, + ConstantExpression.of("Green"))) + .build()) + .build(); +``` + +**Generated SQL (Nested):** +```sql +SELECT document -> 'props' -> 'colors' AS "props.colors" +FROM collection +WHERE document -> 'props' -> 'colors' @> '["Green"]'::jsonb +``` + +### Flat Collections (Currently Not Supported) +For flat collections where `props` is a JSONB column and `item`, `price` are regular columns: + +**Desired Usage:** +```java +Query query = Query.builder() + .addSelection(JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY"), + CONTAINS, + ConstantExpression.of("Green"))) + .build(); +``` + +**Desired SQL (Flat):** +```sql +SELECT props -> 'colors' AS colors +FROM collection +WHERE props -> 'colors' @> '["Green"]'::jsonb +``` + +## Filter Types Analysis + +### 1. CONTAINS Operator + +#### **Use Case: Array Contains Primitive Value** +- **LHS:** Array field (e.g., `props.colors`, `props.brands`) +- **RHS:** Primitive string value +- **Logic:** Check if array contains the value + +**For Nested Collections:** +```java +// Current implementation: PostgresContainsRelationalFilterParser +parsedLhs = "document -> 'props' -> 'colors'" +parsedRhs = "Green" +convertedRhs = "[\"Green\"]" // Wrapped in array + +SQL: document -> 'props' -> 'colors' @> '["Green"]'::jsonb +``` + +**For Flat Collections (Needed):** +```java +parsedLhs = "props -> 'colors'" // From JsonIdentifierExpression +parsedRhs = "Green" +convertedRhs = "[\"Green\"]" + +SQL: props -> 'colors' @> '["Green"]'::jsonb +``` + +**Key Insight:** The same logic works! We just need the LHS parser to handle `JsonIdentifierExpression`. + +--- + +### 2. IN Operator + +#### **Use Case: Scalar IN Array** +- **LHS:** Primitive field (e.g., `id`, `item`) +- **RHS:** Array of values +- **Logic:** Check if LHS value is in RHS array + +**For Nested Collections:** +```java +// Current implementation: PostgresInRelationalFilterParser +// Handles both: scalar IN array AND array-array intersection + +SQL for scalar: +((jsonb_typeof(to_jsonb(LHS)) = 'array' AND to_jsonb(LHS) @> jsonb_build_array(?)) + OR (jsonb_build_array(LHS) @> jsonb_build_array(?))) +``` + +**For Flat Collections (Needed):** +```java +// If LHS is JsonIdentifierExpression pointing to a scalar JSON field +parsedLhs = "props -> 'brand'" // e.g., nested string field + +SQL: Same as above, should work +``` + +**Key Insight:** Current IN parser handles both cases via type checking. Should work for JSON fields too. + +--- + +### 3. NOT_CONTAINS Operator + +#### **Use Case: Array Does NOT Contain Primitive Value** +- **LHS:** Array field +- **RHS:** Primitive value +- **Logic:** Check if array does NOT contain the value + +**Current implementation:** `PostgresNotContainsRelationalFilterParser` +```sql +NOT (document -> 'props' -> 'colors' @> '["Green"]'::jsonb) +``` + +**For Flat Collections:** Same logic should work with JSON accessor. + +--- + +### 4. Standard Relational Operators (EQ, NE, GT, LTE, etc.) + +**For Nested Collections:** +```java +// PostgresStandardRelationalFilterParser +// Uses casting based on field type + +parsedLhs = "document -> 'props' ->> 'price'" // ->> for text extraction +cast = "(document -> 'props' ->> 'price')::numeric" + +SQL: (document -> 'props' ->> 'price')::numeric > 100 +``` + +**For Flat Collections (Needed):** +```java +parsedLhs = "props ->> 'price'" // JsonIdentifierExpression +cast = "(props ->> 'price')::numeric" + +SQL: (props ->> 'price')::numeric > 100 +``` + +**Key Insight:** Standard operators should work once we have proper accessor syntax from JsonIdentifierExpression. + +--- + +## Current Architecture + +### Filter Parser Selection Logic +**File:** `PostgresRelationalFilterParserFactoryImpl.java` + +```java +boolean isFirstClassField = + postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; + +if (expression.getOperator() == CONTAINS) { + return isFirstClassField ? nonJsonFieldContainsParser : jsonFieldContainsParser; +} else if (expression.getOperator() == IN) { + return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; +} +``` + +**Problem:** This checks if the *collection* is flat, not if the *field* is a JSON field within a flat collection. + +### Current Parsers + +| Operator | Nested Collection (JSON) | Flat Collection (Non-JSON) | +|----------|--------------------------|----------------------------| +| CONTAINS | `PostgresContainsRelationalFilterParser` | `PostgresContainsRelationalFilterParserNonJsonField` | +| IN | `PostgresInRelationalFilterParser` | `PostgresInRelationalFilterParserNonJsonField` | +| NOT_CONTAINS | `PostgresNotContainsRelationalFilterParser` | (same) | +| Standard (EQ, GT, etc.) | `PostgresStandardRelationalFilterParser` | (same) | + +--- + +## Gap Analysis + +### What's Missing for Flat Collections + JSON Fields? + +1. **Parser Selection Logic:** + - Currently: Collection type determines parser + - Needed: Field type determines parser + - For `JsonIdentifierExpression` in flat collections → use JSON parsers + +2. **LHS Visitor Support:** + - `PostgresFieldIdentifierExpressionVisitor` needs to handle `JsonIdentifierExpression` + - Should produce: `props -> 'fieldName'` or `props ->> 'fieldName'` + - Already implemented in recent commits for SELECT, needs verification for filters + +3. **No New Parsers Needed:** + - Existing JSON parsers (`PostgresContainsRelationalFilterParser`, `PostgresInRelationalFilterParser`) should work + - Just need to route `JsonIdentifierExpression` to JSON parsers instead of non-JSON parsers + +--- + +## Proposed Solution + +### Option 1: Field-Based Parser Selection (Recommended) + +**Change:** Determine parser based on **LHS expression type**, not collection type. + +```java +// In PostgresRelationalFilterParserFactoryImpl +@Override +public PostgresRelationalFilterParser parser( + final RelationalExpression expression, + final PostgresQueryParser postgresQueryParser) { + + // Check if LHS is a JsonIdentifierExpression (nested JSON field in flat collection) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + + // Check if collection is flat (all fields are first-class columns) + boolean isFlatCollection = + postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; + + // Use JSON parsers for: + // 1. Nested collections (entire document is JSON) + // 2. JSON fields within flat collections (JsonIdentifierExpression) + boolean useJsonParser = !isFlatCollection || isJsonField; + + if (expression.getOperator() == CONTAINS) { + return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; + } else if (expression.getOperator() == IN) { + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; + } + + return parserMap.getOrDefault( + expression.getOperator(), + postgresStandardRelationalFilterParser); +} +``` + +### Option 2: Visitor Pattern (More Extensible) + +**Change:** Have parsers use visitor pattern to determine field type. + +```java +// LHS expression accepts a type checker visitor +boolean isJsonField = expression.getLhs().accept(new IsJsonFieldVisitor()); +``` + +--- + +## Test Cases Needed + +### 1. CONTAINS on JSON Array Field in Flat Collection +```java +@Test +void testFlatCollectionJsonArrayContains() { + Query query = Query.builder() + .addSelection(JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY"), + CONTAINS, + ConstantExpression.of("Green"))) + .build(); + + // Expected SQL: + // WHERE props -> 'colors' @> '["Green"]'::jsonb +} +``` + +### 2. NOT_CONTAINS on JSON Array Field +```java +@Test +void testFlatCollectionJsonArrayNotContains() { + Query query = Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("brands"), "STRING_ARRAY"), + NOT_CONTAINS, + ConstantExpression.of("Brand A"))) + .build(); + + // Expected SQL: + // WHERE NOT (props -> 'brands' @> '["Brand A"]'::jsonb) +} +``` + +### 3. IN Operator with JSON Scalar Field +```java +@Test +void testFlatCollectionJsonScalarIn() { + Query query = Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("brand"), "STRING"), + IN, + ConstantExpression.ofStrings(List.of("Dettol", "Dove")))) + .build(); + + // Expected SQL: + // WHERE ((jsonb_typeof(to_jsonb(props -> 'brand')) = 'array' + // AND to_jsonb(props -> 'brand') @> jsonb_build_array(?)) + // OR (jsonb_build_array(props -> 'brand') @> jsonb_build_array(?))) + // OR ... +} +``` + +### 4. Standard Operator (EQ, GT) on JSON Numeric Field +```java +@Test +void testFlatCollectionJsonNumericComparison() { + Query query = Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("rating"), "LONG"), + GT, + ConstantExpression.of(4))) + .build(); + + // Expected SQL: + // WHERE (props ->> 'rating')::numeric > 4 +} +``` + +### 5. Nested Path (Multiple Levels) +```java +@Test +void testFlatCollectionDeepNestedFilter() { + Query query = Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", List.of("seller", "address", "city"), "STRING"), + EQ, + ConstantExpression.of("New York"))) + .build(); + + // Expected SQL: + // WHERE props -> 'seller' -> 'address' ->> 'city' = 'New York' +} +``` + +--- + +## Implementation Checklist + +- [ ] 1. Update `PostgresRelationalFilterParserFactoryImpl.parser()` to check LHS expression type +- [ ] 2. Verify `PostgresFieldIdentifierExpressionVisitor` handles `JsonIdentifierExpression` for filters +- [ ] 3. Add test: CONTAINS on JSON array field +- [ ] 4. Add test: NOT_CONTAINS on JSON array field +- [ ] 5. Add test: IN operator with JSON scalar field +- [ ] 6. Add test: Standard operators (EQ, GT, LT) on JSON fields +- [ ] 7. Add test: Nested path filters (multiple levels deep) +- [ ] 8. Add consistency test: Flat vs Nested collection same filter results + +--- + +## Key Insights + +1. **No new parsers needed** - Existing JSON parsers work for flat collections too +2. **Parser selection is the key** - Need to route based on field type, not just collection type +3. **LHS visitor already handles JsonIdentifierExpression** - Recent commits added `visit(JsonIdentifierExpression)` +4. **Same SQL patterns** - `props -> 'field'` vs `document -> 'props' -> 'field'` +5. **Type casting already works** - `PostgresDataAccessorIdentifierExpressionVisitor` handles type conversion + +--- + +## Risks & Considerations + +1. **Breaking Changes:** Need to ensure existing flat collection filters (on first-class columns) still use non-JSON parsers +2. **Performance:** JSON filters might be slower than direct column filters +3. **Indexing:** May need JSONB GIN indexes on `props` column for performance +4. **Type Safety:** Ensure type parameter in JsonIdentifierExpression is used correctly for casting + +--- + +## References + +- `PostgresContainsRelationalFilterParser.java` - JSON CONTAINS logic using `@>` operator +- `PostgresInRelationalFilterParser.java` - JSON IN logic with type checking +- `PostgresRelationalFilterParserFactoryImpl.java` - Parser selection based on collection/field type +- `FlatPostgresFieldTransformer.java` - Field transformation for flat collections +- `PostgresFieldIdentifierExpressionVisitor.java` - LHS expression visitor for filters diff --git a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java index cb192c4b..3a70b28c 100644 --- a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java +++ b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java @@ -3981,6 +3981,239 @@ void testFlatVsNestedCollectionNestedFieldSelections(String dataStoreName) throw assertDocsAndSizeEqual( dataStoreName, flatBrandNoAliasIterator, "query/no_alias_response.json", 8); } + + /** + * Tests for JSON field filters on flat PostgreSQL collections. These tests validate filtering + * on nested fields within JSONB columns using JsonIdentifierExpression. + * + *

Currently DISABLED as JsonIdentifierExpression filters are not yet supported for flat + * collections. These tests document the expected behavior and will be enabled once the feature + * is implemented. + */ + @Nested + class FlatCollectionJsonFieldFilters { + + /** + * Tests CONTAINS operator on a JSON array field within a JSONB column. Validates that we can + * filter documents where a JSON array contains a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldContainsFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.colors CONTAINS "Green" + // Expected: Document where colors array contains "Green" (doc 1 only) + Query containsQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(IdentifierExpression.of("price")) + .addSelection(JsonIdentifierExpression.of("props", "colors")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "colors"), + CONTAINS, + ConstantExpression.of("Green"))) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(containsQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_contains_filter_response.json", 1); + } + + /** + * Tests NOT_CONTAINS operator on a JSON array field. Validates that we can filter documents + * where a JSON array does NOT contain a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldNotContainsFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.colors NOT_CONTAINS "Green" AND _id <= 8 + // Note: NOT_CONTAINS with IS NULL logic includes documents where props.colors is NULL or + // doesn't contain "Green" + // Expected: All documents except doc 1 (which contains "Green") and docs 9-10 (excluded by + // _id <= 8) + Query notContainsQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(JsonIdentifierExpression.of("props", "colors")) + .setFilter( + LogicalExpression.builder() + .operator(LogicalOperator.AND) + .operand( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "colors"), + NOT_CONTAINS, + ConstantExpression.of("Green"))) + .operand( + RelationalExpression.of( + IdentifierExpression.of("_id"), LTE, ConstantExpression.of(8))) + .build()) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(notContainsQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_not_contains_filter_response.json", 7); + } + + /** + * Tests IN operator on a scalar JSON field. Validates filtering where a JSON string field's + * value is in a list of values. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldInFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.brand IN ["Dettol", "Lifebuoy"] + // Expected: Documents with brand = "Dettol" (doc 1) or "Lifebuoy" (doc 5) + Query inQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(JsonIdentifierExpression.of("props", "brand")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + IN, + ConstantExpression.ofStrings(List.of("Dettol", "Lifebuoy")))) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(inQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_in_filter_response.json", 2); + } + + /** + * Tests standard equality (EQ) operator on a scalar JSON field. Validates filtering where a + * JSON string field equals a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldEqualityFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.brand EQ "Dettol" + // Expected: Document with brand = "Dettol" (doc 1) + Query eqQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(IdentifierExpression.of("price")) + .addSelection(JsonIdentifierExpression.of("props", "brand")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + EQ, + ConstantExpression.of("Dettol"))) + .build(); + + Iterator resultIterator = flatCollection.find(eqQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_eq_filter_response.json", 1); + } + + /** + * Tests NOT_EQUALS (NEQ) operator on a scalar JSON field. Validates filtering where a JSON + * string field does not equal a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldNotEqualFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.brand NEQ "Dettol" + // Expected: Documents with brand != "Dettol" (docs with Sunsilk and Lifebuoy brands) + Query neqQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(JsonIdentifierExpression.of("props", "brand")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + NEQ, + ConstantExpression.of("Dettol"))) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(neqQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_neq_filter_response.json", 2); + } + + /** + * Tests GREATER_THAN (GT) operator on a nested numeric JSON field. Validates filtering where + * a deeply nested numeric field is greater than a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldGreaterThanFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.seller.address.pincode GT 500000 + // Expected: Documents with pincode > 500000 (docs 5 and 7 with pincode 700007 in Kolkata) + Query gtQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(JsonIdentifierExpression.of("props", "seller", "address", "pincode")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + GT, + ConstantExpression.of(500000))) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(gtQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_gt_filter_response.json", 2); + } + + /** + * Tests LESS_THAN (LT) operator on a nested numeric JSON field. Validates filtering where a + * deeply nested numeric field is less than a specific value. + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testFlatCollectionJsonFieldLessThanFilter(String dataStoreName) throws IOException { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Filter: props.seller.address.pincode LT 500000 + // Expected: Documents with pincode < 500000 (docs 1 and 3 with pincode 400004 in Mumbai) + Query ltQuery = + Query.builder() + .addSelection(IdentifierExpression.of("item")) + .addSelection(JsonIdentifierExpression.of("props", "seller", "address", "pincode")) + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + LT, + ConstantExpression.of(500000))) + .addSort(IdentifierExpression.of("_id"), ASC) + .build(); + + Iterator resultIterator = flatCollection.find(ltQuery); + assertDocsAndSizeEqual( + dataStoreName, resultIterator, "query/flat_json_lt_filter_response.json", 2); + } + } } @Nested diff --git a/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json new file mode 100644 index 00000000..a999c69a --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json @@ -0,0 +1,12 @@ +[ + { + "item": "Soap", + "price": 10, + "props": { + "colors": [ + "Blue", + "Green" + ] + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json new file mode 100644 index 00000000..8dcc744c --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json @@ -0,0 +1,9 @@ +[ + { + "item": "Soap", + "price": 10, + "props": { + "brand": "Dettol" + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json new file mode 100644 index 00000000..946e6acf --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json @@ -0,0 +1,22 @@ +[ + { + "item": "Soap", + "props": { + "seller": { + "address": { + "pincode": 700007 + } + } + } + }, + { + "item": "Comb", + "props": { + "seller": { + "address": { + "pincode": 700007 + } + } + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json new file mode 100644 index 00000000..30495cd0 --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json @@ -0,0 +1,14 @@ +[ + { + "item": "Soap", + "props": { + "brand": "Dettol" + } + }, + { + "item": "Soap", + "props": { + "brand": "Lifebuoy" + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json new file mode 100644 index 00000000..0fac9473 --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json @@ -0,0 +1,22 @@ +[ + { + "item": "Soap", + "props": { + "seller": { + "address": { + "pincode": 400004 + } + } + } + }, + { + "item": "Shampoo", + "props": { + "seller": { + "address": { + "pincode": 400004 + } + } + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json new file mode 100644 index 00000000..4db8824c --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json @@ -0,0 +1,14 @@ +[ + { + "item": "Shampoo", + "props": { + "brand": "Sunsilk" + } + }, + { + "item": "Soap", + "props": { + "brand": "Lifebuoy" + } + } +] diff --git a/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json new file mode 100644 index 00000000..7b3e64bb --- /dev/null +++ b/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json @@ -0,0 +1,37 @@ +[ + { + "item": "Mirror" + }, + { + "item": "Shampoo", + "props": { + "colors": [ + "Black" + ] + } + }, + { + "item": "Shampoo" + }, + { + "item": "Soap", + "props": { + "colors": [ + "Orange", + "Blue" + ] + } + }, + { + "item": "Comb" + }, + { + "item": "Comb", + "props": { + "colors": [] + } + }, + { + "item": "Soap" + } +] diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java index 6d3abf4f..370c724d 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java @@ -68,6 +68,19 @@ public T accept(final FieldTransformationVisitor visitor) { return visitor.visit(this); } + /** + * Accepts a select type expression visitor. Overrides the parent to dispatch to the + * JsonIdentifierExpression-specific visit method. + * + * @param visitor The select type expression visitor + * @param The return type + * @return The result of visiting this expression + */ + @Override + public T accept(final org.hypertrace.core.documentstore.parser.SelectTypeExpressionVisitor visitor) { + return visitor.visit(this); + } + @Override public String toString() { return String.format( diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java index a6e0ed0e..bf0b7375 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java @@ -6,6 +6,7 @@ import org.hypertrace.core.documentstore.expression.impl.ConstantExpression.DocumentConstantExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; public interface SelectTypeExpressionVisitor { T visit(final AggregateExpression expression); @@ -19,4 +20,8 @@ public interface SelectTypeExpressionVisitor { T visit(final IdentifierExpression expression); T visit(final AliasedIdentifierExpression expression); + + default T visit(final JsonIdentifierExpression expression) { + return visit((IdentifierExpression) expression); + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java index d2c444a9..2bce8e04 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; @@ -16,16 +17,25 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - boolean isFirstClassField = - context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - if (isFirstClassField) { - // Use the non-JSON logic for first-class fields - String containsExpression = nonJsonContainsParser.parse(expression, context); - return String.format("%s IS NULL OR NOT (%s)", parsedLhs, containsExpression); - } else { - // Use the JSON logic for document fields. + // Check if LHS is a JSON field (JSONB column access) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + + // Check if the collection type is flat or nested + boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + // Use JSON parser for: + // 1. Nested collections - !isFlatCollection + // 2. JSON fields within flat collections - isJsonField + boolean useJsonParser = !isFlatCollection || isJsonField; + + if (useJsonParser) { + // Use the JSON logic for JSON document fields jsonContainsParser.parse(expression, context); // This adds the parameter. return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); + } else { + // Use the non-JSON logic for first-class array fields + String containsExpression = nonJsonContainsParser.parse(expression, context); + return String.format("%s IS NULL OR NOT (%s)", parsedLhs, containsExpression); } } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java index 91c80986..ce204c5b 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; @@ -16,17 +17,26 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - PostgresInRelationalFilterParserInterface inFilterParser = getInFilterParser(context); + PostgresInRelationalFilterParserInterface inFilterParser = + getInFilterParser(expression, context); final String parsedInExpression = inFilterParser.parse(expression, context); return String.format("%s IS NULL OR NOT (%s)", parsedLhs, parsedInExpression); } private PostgresInRelationalFilterParserInterface getInFilterParser( - PostgresRelationalFilterContext context) { - boolean isFirstClassField = - context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + final RelationalExpression expression, PostgresRelationalFilterContext context) { + // Check if LHS is a JSON field (JSONB column access) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; + // Check if the collection type is flat or nested + boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + // Use JSON parser for: + // 1. Nested collections - !isFlatCollection + // 2. JSON fields within flat collections - isJsonField + boolean useJsonParser = !isFlatCollection || isJsonField; + + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java index 0fd575b8..725d3aa2 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java @@ -12,15 +12,17 @@ import com.google.common.collect.Maps; import java.util.Map; +import org.hypertrace.core.documentstore.DocumentType; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.expression.operators.RelationalOperator; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; -import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FlatPostgresFieldTransformer; public class PostgresRelationalFilterParserFactoryImpl implements PostgresRelationalFilterParserFactory { + private static final Map parserMap = Maps.immutableEnumMap( Map.ofEntries( @@ -52,15 +54,39 @@ public class PostgresRelationalFilterParserFactoryImpl public PostgresRelationalFilterParser parser( final RelationalExpression expression, final PostgresQueryParser postgresQueryParser) { - boolean isFirstClassField = - postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; + // Determine if we should use JSON-specific parsers based on: + // 1. Collection type (nested collections store entire document as JSONB) + // 2. Field type (flat collections may have JSONB columns for nested data) + boolean useJsonParser = shouldUseJsonParser(expression, postgresQueryParser); if (expression.getOperator() == CONTAINS) { - return isFirstClassField ? nonJsonFieldContainsParser : jsonFieldContainsParser; + return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; } else if (expression.getOperator() == IN) { - return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; } return parserMap.getOrDefault(expression.getOperator(), postgresStandardRelationalFilterParser); } + + /** + * Determines if JSON-specific parser should be used based on collection type and field type. + * + * @param expression the relational expression to evaluate + * @param postgresQueryParser the query parser context + * @return true if JSON parser should be used, false for standard SQL parser + */ + private boolean shouldUseJsonParser( + final RelationalExpression expression, final PostgresQueryParser postgresQueryParser) { + // Check if the collection type is flat or nested + boolean isFlatCollection = + postgresQueryParser.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + // Check if LHS is a JSON field (JSONB column access) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + + // Use JSON parsers for: + // 1. Nested collections (entire document is JSONB) - !isFlatCollection + // 2. JSON fields within flat collections - isJsonField + return !isFlatCollection || isJsonField; + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java index 7052b90a..aace4e97 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java @@ -10,6 +10,7 @@ import org.hypertrace.core.documentstore.expression.impl.ConstantExpression.DocumentConstantExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.type.GroupTypeExpression; import org.hypertrace.core.documentstore.expression.type.SelectTypeExpression; import org.hypertrace.core.documentstore.parser.GroupTypeExpressionVisitor; @@ -111,6 +112,11 @@ public Boolean visit(IdentifierExpression expression) { return false; } + @Override + public Boolean visit(JsonIdentifierExpression expression) { + return false; + } + @Override public Boolean visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); @@ -144,6 +150,11 @@ public Boolean visit(IdentifierExpression expression) { return true; } + @Override + public Boolean visit(JsonIdentifierExpression expression) { + return true; + } + @Override public Boolean visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java index 8145b8f7..75876d0d 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java @@ -16,6 +16,7 @@ import org.hypertrace.core.documentstore.expression.impl.DocumentArrayFilterExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.KeyExpression; import org.hypertrace.core.documentstore.expression.impl.LogicalExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; @@ -219,6 +220,11 @@ public List visit(IdentifierExpression expression) { return List.of(expression.getName()); } + @Override + public List visit(JsonIdentifierExpression expression) { + return List.of(expression.getName()); + } + @Override public List visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java index 44602b45..a3f319d7 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java @@ -2,6 +2,7 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FieldToPgColumn; import org.hypertrace.core.documentstore.postgres.utils.PostgresUtils.Type; @@ -48,4 +49,15 @@ public String visit(final IdentifierExpression expression) { .getPgColTransformer() .buildFieldAccessorWithCast(fieldToPgColumn, this.type); } + + @Override + public String visit(final JsonIdentifierExpression expression) { + FieldToPgColumn fieldToPgColumn = getPostgresQueryParser().transformField(expression); + + // Type parameter is ignored for JSON fields - always returns JSONB + // buildFieldAccessorWithCast will use -> for all accessors without casting + return getPostgresQueryParser() + .getPgColTransformer() + .buildFieldAccessorWithCast(fieldToPgColumn, this.type); + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java index 189361ba..77f9fe4f 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java @@ -2,6 +2,7 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FieldToPgColumn; @@ -28,4 +29,12 @@ public String visit(final IdentifierExpression expression) { .getPgColTransformer() .buildFieldAccessorWithoutCast(fieldToPgColumn); } + + @Override + public String visit(final JsonIdentifierExpression expression) { + FieldToPgColumn fieldToPgColumn = getPostgresQueryParser().transformField(expression); + return getPostgresQueryParser() + .getPgColTransformer() + .buildFieldAccessorWithoutCast(fieldToPgColumn); + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java index 1fe4ae1d..8402f5c7 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java @@ -2,6 +2,7 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; @NoArgsConstructor @@ -24,4 +25,9 @@ public PostgresQueryParser getPostgresQueryParser() { public String visit(final IdentifierExpression expression) { return expression.getName(); } + + @Override + public String visit(final JsonIdentifierExpression expression) { + return expression.getName(); + } } diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java index fe827257..cc963152 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.expression.impl; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -125,4 +126,38 @@ void testOfVarargsWithNull() { assertEquals("JSON path cannot be null or empty", exception.getMessage()); } + + /** + * Tests instanceof check - JsonIdentifierExpression should be identifiable. + */ + @Test + void testInstanceOfJsonIdentifierExpression() { + JsonIdentifierExpression jsonExpr = JsonIdentifierExpression.of("props", "brand"); + + // Should be identifiable as JsonIdentifierExpression + assertTrue(jsonExpr instanceof JsonIdentifierExpression, "Should be instanceof JsonIdentifierExpression"); + } + + /** + * Tests instanceof check through base class reference. + */ + @Test + void testInstanceOfThroughBaseReference() { + // Create JsonIdentifierExpression but hold as IdentifierExpression reference + IdentifierExpression expr = JsonIdentifierExpression.of("props", "brand"); + + // Should still work with instanceof check + assertTrue(expr instanceof JsonIdentifierExpression, "instanceof should work through base class reference"); + } + + /** + * Tests that regular IdentifierExpression is not JsonIdentifierExpression. + */ + @Test + void testRegularIdentifierIsNotJsonIdentifier() { + IdentifierExpression expr = IdentifierExpression.of("regularColumn"); + + // Regular IdentifierExpression should not be JsonIdentifierExpression + assertFalse(expr instanceof JsonIdentifierExpression, "Regular identifier should not be instanceof JsonIdentifierExpression"); + } } diff --git a/settings-gradle.lockfile b/settings-gradle.lockfile new file mode 100644 index 00000000..709a43f7 --- /dev/null +++ b/settings-gradle.lockfile @@ -0,0 +1,4 @@ +# This is a Gradle generated file for dependency locking. +# Manual edits can break the build and are not advised. +# This file is expected to be part of source control. +empty=incomingCatalogForLibs0 From 9e4000fab7a536ea696d4af6322cacb44519e9f9 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Wed, 22 Oct 2025 11:42:16 +0530 Subject: [PATCH 2/5] Revert "WIP" This reverts commit a333e897a91b2097d7049aadd924cc73f35be793. --- JSONB_FILTER_ARCHITECTURE.md | 219 ---------- NESTED_FILTER_ANALYSIS.md | 382 ------------------ .../documentstore/DocStoreQueryV1Test.java | 233 ----------- .../flat_json_contains_filter_response.json | 12 - .../query/flat_json_eq_filter_response.json | 9 - .../query/flat_json_gt_filter_response.json | 22 - .../query/flat_json_in_filter_response.json | 14 - .../query/flat_json_lt_filter_response.json | 22 - .../query/flat_json_neq_filter_response.json | 14 - ...lat_json_not_contains_filter_response.json | 37 -- .../impl/JsonIdentifierExpression.java | 13 - .../parser/SelectTypeExpressionVisitor.java | 5 - ...gresNotContainsRelationalFilterParser.java | 26 +- .../PostgresNotInRelationalFilterParser.java | 20 +- ...gresRelationalFilterParserFactoryImpl.java | 36 +- .../PostgresSelectionQueryTransformer.java | 11 - .../PostgresUnnestQueryTransformer.java | 6 - ...taAccessorIdentifierExpressionVisitor.java | 12 - ...tgresFieldIdentifierExpressionVisitor.java | 9 - .../PostgresIdentifierExpressionVisitor.java | 6 - .../impl/JsonIdentifierExpressionTest.java | 35 -- settings-gradle.lockfile | 4 - 22 files changed, 18 insertions(+), 1129 deletions(-) delete mode 100644 JSONB_FILTER_ARCHITECTURE.md delete mode 100644 NESTED_FILTER_ANALYSIS.md delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json delete mode 100644 document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json delete mode 100644 settings-gradle.lockfile diff --git a/JSONB_FILTER_ARCHITECTURE.md b/JSONB_FILTER_ARCHITECTURE.md deleted file mode 100644 index ef3de3ac..00000000 --- a/JSONB_FILTER_ARCHITECTURE.md +++ /dev/null @@ -1,219 +0,0 @@ -# JSONB Filter Architecture - Simple instanceof Design - -## Overview - -This document describes the architecture used to handle CONTAINS and IN filters on JSONB nested columns using simple, direct `instanceof` type checking. - -## Architecture Components - -### 1. **Type Detection via instanceof** - -```java -// Simple, direct type check -boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; -``` - -**Purpose:** Determines if an expression represents a JSON field access (JSONB column). - -**Benefits:** -- ✅ **Simple & Direct** - Immediately clear what's being checked -- ✅ **Standard Java** - Everyone understands instanceof -- ✅ **No extra methods** - Doesn't pollute interfaces -- ✅ **Type-safe** - Compile-time type checking -- ✅ **Appropriate** - Right tool for type checking - -### 2. **Factory Pattern for Parser Selection** - -#### `PostgresRelationalFilterParserFactoryImpl` -```java -// Location: postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java - -public PostgresRelationalFilterParser parser( - RelationalExpression expression, - PostgresQueryParser postgresQueryParser) { - - // Determine parser type based on collection and field type - boolean useJsonParser = shouldUseJsonParser(expression, postgresQueryParser); - - if (expression.getOperator() == CONTAINS) { - return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; - } else if (expression.getOperator() == IN) { - return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; - } - // ... -} - -private boolean shouldUseJsonParser( - RelationalExpression expression, - PostgresQueryParser postgresQueryParser) { - - boolean isFlatCollection = - postgresQueryParser.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - - // Simple instanceof check - boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - - return !isFlatCollection || isJsonField; -} -``` - -**Decision Logic:** -``` -Use JSON Parser when: -1. Nested collections (entire document is JSONB), OR -2. JSON fields within flat collections (JSONB columns) - -Use Standard Parser when: -- Flat collections with first-class columns (e.g., text[], int) -``` - -### 3. **Parser Implementations** - -#### For JSON Fields (JSONB columns) -- **`PostgresContainsRelationalFilterParser`** - Uses `@>` with JSONB casting -- **`PostgresInRelationalFilterParser`** - Uses `jsonb_typeof()` and `@>` operators - -```sql --- CONTAINS on JSON field -props @> '["value"]'::jsonb - --- IN on JSON field -(jsonb_typeof(to_jsonb(props)) = 'array' AND to_jsonb(props) @> jsonb_build_array(?)) -``` - -#### For Non-JSON Fields (Regular PostgreSQL arrays) -- **`PostgresContainsRelationalFilterParserNonJsonField`** - Uses array `@>` operator -- **`PostgresInRelationalFilterParserNonJsonField`** - Uses array `&&` operator - -```sql --- CONTAINS on array column -tags_unnested @> ARRAY[?]::text[] - --- IN on array column -ARRAY[tags_unnested]::text[] && ARRAY[?, ?]::text[] -``` - -## Design Principles - -### ✅ **1. Pragmatic Simplicity** -```java -// Direct type checking - simple and clear -boolean isJsonField = expression instanceof JsonIdentifierExpression; -``` - -**Why instanceof is appropriate here:** -- **Clarity** - Intent is immediately obvious -- **Standard** - Well-known Java idiom -- **Appropriate** - This is exactly what instanceof is for -- **KISS** - Keep It Simple, Stupid -- **YAGNI** - You Aren't Gonna Need It (no extra abstraction) - -### ✅ **2. Single Responsibility** -- **Expression classes** - Represent field access patterns -- **Factory** - Selects appropriate parser based on field type -- **Parsers** - Generate SQL for specific operators and field types -- Each class does one thing well - -### ✅ **3. Type Safety** -- `instanceof` is compile-time type-safe -- Type hierarchy provides structure -- No runtime casting errors in normal flow - -### ✅ **4. Easy to Extend** -New expression types can be added by: -1. Create new expression class extending `IdentifierExpression` -2. Factory automatically uses correct parser based on `instanceof` check -3. Add new `instanceof` check only if special handling needed - -## Extension Points - -### Adding New Operators - -1. **Create parser interface:** -```java -interface PostgresMyOperatorParserInterface extends PostgresRelationalFilterParser { - String parse(RelationalExpression expression, PostgresRelationalFilterContext context); -} -``` - -2. **Create JSON and non-JSON implementations:** -```java -class PostgresMyOperatorParser implements PostgresMyOperatorParserInterface { } -class PostgresMyOperatorParserNonJsonField implements PostgresMyOperatorParserInterface { } -``` - -3. **Update factory:** -```java -if (expression.getOperator() == MY_OPERATOR) { - return useJsonParser ? jsonFieldMyOperatorParser : nonJsonFieldMyOperatorParser; -} -``` - -### Adding New Expression Types - -1. **Create the new expression class:** -```java -public class MyNewExpression extends IdentifierExpression { - // ... your implementation -} -``` - -2. **Add instanceof check in factory if special handling needed:** -```java -if (expression.getLhs() instanceof MyNewExpression) { - // Use special parser for this type -} -``` - -That's it! No method overriding needed unless special behavior is required. - -## Testing Strategy - -### Unit Tests -- Factory selects correct parser based on field type -- Each parser generates correct SQL -- Type detection works correctly for all expression types - -### Integration Tests -- CONTAINS filters work on JSONB columns -- IN filters work on JSONB columns -- Mixed queries with JSON and non-JSON fields -- Nested vs flat collection behavior - -## Summary - -**The architecture uses simple instanceof checking:** -1. ✅ Direct `instanceof JsonIdentifierExpression` type checks -2. ✅ Factory pattern for parser selection based on field type -3. ✅ Clean separation between JSON and non-JSON parsers -4. ✅ No unnecessary abstraction layers - -**Key Benefits:** -- ✅ **Simple** - Immediately understandable -- ✅ **Direct** - No indirection or extra methods -- ✅ **Standard Java** - Uses well-known idioms -- ✅ **Type-Safe** - Compile-time checking -- ✅ **Appropriate** - Right tool for the job -- ✅ **No pollution** - Doesn't add methods to wrong interfaces - -**Why instanceof is Best Here:** -- ✅ Type checking is exactly what `instanceof` is for -- ✅ Only 3 places in code that need the check -- ✅ Not a "type property" - just implementation detail -- ✅ Simpler than any abstraction (KISS principle) -- ✅ Easier to understand and maintain - -**Comparison with Alternatives:** - -| Approach | Pros | Cons | Verdict | -|----------|------|------|---------| -| **instanceof** ✅ | Simple, direct, clear | Some say "not OOP" | **Best for this case** | -| Polymorphic Method | "More OOP" | Pollutes interfaces, over-engineered | Overkill | -| Visitor Pattern | Very extensible | Way too complex for simple check | Overkill | -| Marker Interface | Type-safe | Still uses instanceof | Adds complexity | - -**When to use each pattern:** -- **instanceof** ✅ - Simple type checks (this case) -- **Polymorphic Method** - Behavior that's core to type contract -- **Visitor Pattern** - Multiple complex operations across types -- **Marker Interface** - Semantic grouping of multiple types diff --git a/NESTED_FILTER_ANALYSIS.md b/NESTED_FILTER_ANALYSIS.md deleted file mode 100644 index 6dfd48b1..00000000 --- a/NESTED_FILTER_ANALYSIS.md +++ /dev/null @@ -1,382 +0,0 @@ -# Analysis: Nested JSON Field Filters for Flat PostgreSQL Collections - -## Current State - -### Nested Collections (Working) -For nested PostgreSQL collections where the entire document is stored in a JSONB column (`document`), filters on nested fields like `props.colors` work: - -```java -// Test case: testContains (line 1527-1547) -Query query = Query.builder() - .addSelection(IdentifierExpression.of("props.colors")) - .setFilter( - Filter.builder() - .expression( - RelationalExpression.of( - IdentifierExpression.of("props.colors"), - CONTAINS, - ConstantExpression.of("Green"))) - .build()) - .build(); -``` - -**Generated SQL (Nested):** -```sql -SELECT document -> 'props' -> 'colors' AS "props.colors" -FROM collection -WHERE document -> 'props' -> 'colors' @> '["Green"]'::jsonb -``` - -### Flat Collections (Currently Not Supported) -For flat collections where `props` is a JSONB column and `item`, `price` are regular columns: - -**Desired Usage:** -```java -Query query = Query.builder() - .addSelection(JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY"), - CONTAINS, - ConstantExpression.of("Green"))) - .build(); -``` - -**Desired SQL (Flat):** -```sql -SELECT props -> 'colors' AS colors -FROM collection -WHERE props -> 'colors' @> '["Green"]'::jsonb -``` - -## Filter Types Analysis - -### 1. CONTAINS Operator - -#### **Use Case: Array Contains Primitive Value** -- **LHS:** Array field (e.g., `props.colors`, `props.brands`) -- **RHS:** Primitive string value -- **Logic:** Check if array contains the value - -**For Nested Collections:** -```java -// Current implementation: PostgresContainsRelationalFilterParser -parsedLhs = "document -> 'props' -> 'colors'" -parsedRhs = "Green" -convertedRhs = "[\"Green\"]" // Wrapped in array - -SQL: document -> 'props' -> 'colors' @> '["Green"]'::jsonb -``` - -**For Flat Collections (Needed):** -```java -parsedLhs = "props -> 'colors'" // From JsonIdentifierExpression -parsedRhs = "Green" -convertedRhs = "[\"Green\"]" - -SQL: props -> 'colors' @> '["Green"]'::jsonb -``` - -**Key Insight:** The same logic works! We just need the LHS parser to handle `JsonIdentifierExpression`. - ---- - -### 2. IN Operator - -#### **Use Case: Scalar IN Array** -- **LHS:** Primitive field (e.g., `id`, `item`) -- **RHS:** Array of values -- **Logic:** Check if LHS value is in RHS array - -**For Nested Collections:** -```java -// Current implementation: PostgresInRelationalFilterParser -// Handles both: scalar IN array AND array-array intersection - -SQL for scalar: -((jsonb_typeof(to_jsonb(LHS)) = 'array' AND to_jsonb(LHS) @> jsonb_build_array(?)) - OR (jsonb_build_array(LHS) @> jsonb_build_array(?))) -``` - -**For Flat Collections (Needed):** -```java -// If LHS is JsonIdentifierExpression pointing to a scalar JSON field -parsedLhs = "props -> 'brand'" // e.g., nested string field - -SQL: Same as above, should work -``` - -**Key Insight:** Current IN parser handles both cases via type checking. Should work for JSON fields too. - ---- - -### 3. NOT_CONTAINS Operator - -#### **Use Case: Array Does NOT Contain Primitive Value** -- **LHS:** Array field -- **RHS:** Primitive value -- **Logic:** Check if array does NOT contain the value - -**Current implementation:** `PostgresNotContainsRelationalFilterParser` -```sql -NOT (document -> 'props' -> 'colors' @> '["Green"]'::jsonb) -``` - -**For Flat Collections:** Same logic should work with JSON accessor. - ---- - -### 4. Standard Relational Operators (EQ, NE, GT, LTE, etc.) - -**For Nested Collections:** -```java -// PostgresStandardRelationalFilterParser -// Uses casting based on field type - -parsedLhs = "document -> 'props' ->> 'price'" // ->> for text extraction -cast = "(document -> 'props' ->> 'price')::numeric" - -SQL: (document -> 'props' ->> 'price')::numeric > 100 -``` - -**For Flat Collections (Needed):** -```java -parsedLhs = "props ->> 'price'" // JsonIdentifierExpression -cast = "(props ->> 'price')::numeric" - -SQL: (props ->> 'price')::numeric > 100 -``` - -**Key Insight:** Standard operators should work once we have proper accessor syntax from JsonIdentifierExpression. - ---- - -## Current Architecture - -### Filter Parser Selection Logic -**File:** `PostgresRelationalFilterParserFactoryImpl.java` - -```java -boolean isFirstClassField = - postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; - -if (expression.getOperator() == CONTAINS) { - return isFirstClassField ? nonJsonFieldContainsParser : jsonFieldContainsParser; -} else if (expression.getOperator() == IN) { - return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; -} -``` - -**Problem:** This checks if the *collection* is flat, not if the *field* is a JSON field within a flat collection. - -### Current Parsers - -| Operator | Nested Collection (JSON) | Flat Collection (Non-JSON) | -|----------|--------------------------|----------------------------| -| CONTAINS | `PostgresContainsRelationalFilterParser` | `PostgresContainsRelationalFilterParserNonJsonField` | -| IN | `PostgresInRelationalFilterParser` | `PostgresInRelationalFilterParserNonJsonField` | -| NOT_CONTAINS | `PostgresNotContainsRelationalFilterParser` | (same) | -| Standard (EQ, GT, etc.) | `PostgresStandardRelationalFilterParser` | (same) | - ---- - -## Gap Analysis - -### What's Missing for Flat Collections + JSON Fields? - -1. **Parser Selection Logic:** - - Currently: Collection type determines parser - - Needed: Field type determines parser - - For `JsonIdentifierExpression` in flat collections → use JSON parsers - -2. **LHS Visitor Support:** - - `PostgresFieldIdentifierExpressionVisitor` needs to handle `JsonIdentifierExpression` - - Should produce: `props -> 'fieldName'` or `props ->> 'fieldName'` - - Already implemented in recent commits for SELECT, needs verification for filters - -3. **No New Parsers Needed:** - - Existing JSON parsers (`PostgresContainsRelationalFilterParser`, `PostgresInRelationalFilterParser`) should work - - Just need to route `JsonIdentifierExpression` to JSON parsers instead of non-JSON parsers - ---- - -## Proposed Solution - -### Option 1: Field-Based Parser Selection (Recommended) - -**Change:** Determine parser based on **LHS expression type**, not collection type. - -```java -// In PostgresRelationalFilterParserFactoryImpl -@Override -public PostgresRelationalFilterParser parser( - final RelationalExpression expression, - final PostgresQueryParser postgresQueryParser) { - - // Check if LHS is a JsonIdentifierExpression (nested JSON field in flat collection) - boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - - // Check if collection is flat (all fields are first-class columns) - boolean isFlatCollection = - postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; - - // Use JSON parsers for: - // 1. Nested collections (entire document is JSON) - // 2. JSON fields within flat collections (JsonIdentifierExpression) - boolean useJsonParser = !isFlatCollection || isJsonField; - - if (expression.getOperator() == CONTAINS) { - return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; - } else if (expression.getOperator() == IN) { - return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; - } - - return parserMap.getOrDefault( - expression.getOperator(), - postgresStandardRelationalFilterParser); -} -``` - -### Option 2: Visitor Pattern (More Extensible) - -**Change:** Have parsers use visitor pattern to determine field type. - -```java -// LHS expression accepts a type checker visitor -boolean isJsonField = expression.getLhs().accept(new IsJsonFieldVisitor()); -``` - ---- - -## Test Cases Needed - -### 1. CONTAINS on JSON Array Field in Flat Collection -```java -@Test -void testFlatCollectionJsonArrayContains() { - Query query = Query.builder() - .addSelection(JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("colors"), "STRING_ARRAY"), - CONTAINS, - ConstantExpression.of("Green"))) - .build(); - - // Expected SQL: - // WHERE props -> 'colors' @> '["Green"]'::jsonb -} -``` - -### 2. NOT_CONTAINS on JSON Array Field -```java -@Test -void testFlatCollectionJsonArrayNotContains() { - Query query = Query.builder() - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("brands"), "STRING_ARRAY"), - NOT_CONTAINS, - ConstantExpression.of("Brand A"))) - .build(); - - // Expected SQL: - // WHERE NOT (props -> 'brands' @> '["Brand A"]'::jsonb) -} -``` - -### 3. IN Operator with JSON Scalar Field -```java -@Test -void testFlatCollectionJsonScalarIn() { - Query query = Query.builder() - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("brand"), "STRING"), - IN, - ConstantExpression.ofStrings(List.of("Dettol", "Dove")))) - .build(); - - // Expected SQL: - // WHERE ((jsonb_typeof(to_jsonb(props -> 'brand')) = 'array' - // AND to_jsonb(props -> 'brand') @> jsonb_build_array(?)) - // OR (jsonb_build_array(props -> 'brand') @> jsonb_build_array(?))) - // OR ... -} -``` - -### 4. Standard Operator (EQ, GT) on JSON Numeric Field -```java -@Test -void testFlatCollectionJsonNumericComparison() { - Query query = Query.builder() - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("rating"), "LONG"), - GT, - ConstantExpression.of(4))) - .build(); - - // Expected SQL: - // WHERE (props ->> 'rating')::numeric > 4 -} -``` - -### 5. Nested Path (Multiple Levels) -```java -@Test -void testFlatCollectionDeepNestedFilter() { - Query query = Query.builder() - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", List.of("seller", "address", "city"), "STRING"), - EQ, - ConstantExpression.of("New York"))) - .build(); - - // Expected SQL: - // WHERE props -> 'seller' -> 'address' ->> 'city' = 'New York' -} -``` - ---- - -## Implementation Checklist - -- [ ] 1. Update `PostgresRelationalFilterParserFactoryImpl.parser()` to check LHS expression type -- [ ] 2. Verify `PostgresFieldIdentifierExpressionVisitor` handles `JsonIdentifierExpression` for filters -- [ ] 3. Add test: CONTAINS on JSON array field -- [ ] 4. Add test: NOT_CONTAINS on JSON array field -- [ ] 5. Add test: IN operator with JSON scalar field -- [ ] 6. Add test: Standard operators (EQ, GT, LT) on JSON fields -- [ ] 7. Add test: Nested path filters (multiple levels deep) -- [ ] 8. Add consistency test: Flat vs Nested collection same filter results - ---- - -## Key Insights - -1. **No new parsers needed** - Existing JSON parsers work for flat collections too -2. **Parser selection is the key** - Need to route based on field type, not just collection type -3. **LHS visitor already handles JsonIdentifierExpression** - Recent commits added `visit(JsonIdentifierExpression)` -4. **Same SQL patterns** - `props -> 'field'` vs `document -> 'props' -> 'field'` -5. **Type casting already works** - `PostgresDataAccessorIdentifierExpressionVisitor` handles type conversion - ---- - -## Risks & Considerations - -1. **Breaking Changes:** Need to ensure existing flat collection filters (on first-class columns) still use non-JSON parsers -2. **Performance:** JSON filters might be slower than direct column filters -3. **Indexing:** May need JSONB GIN indexes on `props` column for performance -4. **Type Safety:** Ensure type parameter in JsonIdentifierExpression is used correctly for casting - ---- - -## References - -- `PostgresContainsRelationalFilterParser.java` - JSON CONTAINS logic using `@>` operator -- `PostgresInRelationalFilterParser.java` - JSON IN logic with type checking -- `PostgresRelationalFilterParserFactoryImpl.java` - Parser selection based on collection/field type -- `FlatPostgresFieldTransformer.java` - Field transformation for flat collections -- `PostgresFieldIdentifierExpressionVisitor.java` - LHS expression visitor for filters diff --git a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java index 3a70b28c..cb192c4b 100644 --- a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java +++ b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java @@ -3981,239 +3981,6 @@ void testFlatVsNestedCollectionNestedFieldSelections(String dataStoreName) throw assertDocsAndSizeEqual( dataStoreName, flatBrandNoAliasIterator, "query/no_alias_response.json", 8); } - - /** - * Tests for JSON field filters on flat PostgreSQL collections. These tests validate filtering - * on nested fields within JSONB columns using JsonIdentifierExpression. - * - *

Currently DISABLED as JsonIdentifierExpression filters are not yet supported for flat - * collections. These tests document the expected behavior and will be enabled once the feature - * is implemented. - */ - @Nested - class FlatCollectionJsonFieldFilters { - - /** - * Tests CONTAINS operator on a JSON array field within a JSONB column. Validates that we can - * filter documents where a JSON array contains a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldContainsFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.colors CONTAINS "Green" - // Expected: Document where colors array contains "Green" (doc 1 only) - Query containsQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(IdentifierExpression.of("price")) - .addSelection(JsonIdentifierExpression.of("props", "colors")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "colors"), - CONTAINS, - ConstantExpression.of("Green"))) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(containsQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_contains_filter_response.json", 1); - } - - /** - * Tests NOT_CONTAINS operator on a JSON array field. Validates that we can filter documents - * where a JSON array does NOT contain a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldNotContainsFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.colors NOT_CONTAINS "Green" AND _id <= 8 - // Note: NOT_CONTAINS with IS NULL logic includes documents where props.colors is NULL or - // doesn't contain "Green" - // Expected: All documents except doc 1 (which contains "Green") and docs 9-10 (excluded by - // _id <= 8) - Query notContainsQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(JsonIdentifierExpression.of("props", "colors")) - .setFilter( - LogicalExpression.builder() - .operator(LogicalOperator.AND) - .operand( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "colors"), - NOT_CONTAINS, - ConstantExpression.of("Green"))) - .operand( - RelationalExpression.of( - IdentifierExpression.of("_id"), LTE, ConstantExpression.of(8))) - .build()) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(notContainsQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_not_contains_filter_response.json", 7); - } - - /** - * Tests IN operator on a scalar JSON field. Validates filtering where a JSON string field's - * value is in a list of values. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldInFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.brand IN ["Dettol", "Lifebuoy"] - // Expected: Documents with brand = "Dettol" (doc 1) or "Lifebuoy" (doc 5) - Query inQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(JsonIdentifierExpression.of("props", "brand")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "brand"), - IN, - ConstantExpression.ofStrings(List.of("Dettol", "Lifebuoy")))) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(inQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_in_filter_response.json", 2); - } - - /** - * Tests standard equality (EQ) operator on a scalar JSON field. Validates filtering where a - * JSON string field equals a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldEqualityFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.brand EQ "Dettol" - // Expected: Document with brand = "Dettol" (doc 1) - Query eqQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(IdentifierExpression.of("price")) - .addSelection(JsonIdentifierExpression.of("props", "brand")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "brand"), - EQ, - ConstantExpression.of("Dettol"))) - .build(); - - Iterator resultIterator = flatCollection.find(eqQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_eq_filter_response.json", 1); - } - - /** - * Tests NOT_EQUALS (NEQ) operator on a scalar JSON field. Validates filtering where a JSON - * string field does not equal a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldNotEqualFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.brand NEQ "Dettol" - // Expected: Documents with brand != "Dettol" (docs with Sunsilk and Lifebuoy brands) - Query neqQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(JsonIdentifierExpression.of("props", "brand")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "brand"), - NEQ, - ConstantExpression.of("Dettol"))) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(neqQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_neq_filter_response.json", 2); - } - - /** - * Tests GREATER_THAN (GT) operator on a nested numeric JSON field. Validates filtering where - * a deeply nested numeric field is greater than a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldGreaterThanFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.seller.address.pincode GT 500000 - // Expected: Documents with pincode > 500000 (docs 5 and 7 with pincode 700007 in Kolkata) - Query gtQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(JsonIdentifierExpression.of("props", "seller", "address", "pincode")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "seller", "address", "pincode"), - GT, - ConstantExpression.of(500000))) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(gtQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_gt_filter_response.json", 2); - } - - /** - * Tests LESS_THAN (LT) operator on a nested numeric JSON field. Validates filtering where a - * deeply nested numeric field is less than a specific value. - */ - @ParameterizedTest - @ArgumentsSource(PostgresProvider.class) - void testFlatCollectionJsonFieldLessThanFilter(String dataStoreName) throws IOException { - Datastore datastore = datastoreMap.get(dataStoreName); - Collection flatCollection = - datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); - - // Filter: props.seller.address.pincode LT 500000 - // Expected: Documents with pincode < 500000 (docs 1 and 3 with pincode 400004 in Mumbai) - Query ltQuery = - Query.builder() - .addSelection(IdentifierExpression.of("item")) - .addSelection(JsonIdentifierExpression.of("props", "seller", "address", "pincode")) - .setFilter( - RelationalExpression.of( - JsonIdentifierExpression.of("props", "seller", "address", "pincode"), - LT, - ConstantExpression.of(500000))) - .addSort(IdentifierExpression.of("_id"), ASC) - .build(); - - Iterator resultIterator = flatCollection.find(ltQuery); - assertDocsAndSizeEqual( - dataStoreName, resultIterator, "query/flat_json_lt_filter_response.json", 2); - } - } } @Nested diff --git a/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json deleted file mode 100644 index a999c69a..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_contains_filter_response.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "item": "Soap", - "price": 10, - "props": { - "colors": [ - "Blue", - "Green" - ] - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json deleted file mode 100644 index 8dcc744c..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_eq_filter_response.json +++ /dev/null @@ -1,9 +0,0 @@ -[ - { - "item": "Soap", - "price": 10, - "props": { - "brand": "Dettol" - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json deleted file mode 100644 index 946e6acf..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_gt_filter_response.json +++ /dev/null @@ -1,22 +0,0 @@ -[ - { - "item": "Soap", - "props": { - "seller": { - "address": { - "pincode": 700007 - } - } - } - }, - { - "item": "Comb", - "props": { - "seller": { - "address": { - "pincode": 700007 - } - } - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json deleted file mode 100644 index 30495cd0..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_in_filter_response.json +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - "item": "Soap", - "props": { - "brand": "Dettol" - } - }, - { - "item": "Soap", - "props": { - "brand": "Lifebuoy" - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json deleted file mode 100644 index 0fac9473..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_lt_filter_response.json +++ /dev/null @@ -1,22 +0,0 @@ -[ - { - "item": "Soap", - "props": { - "seller": { - "address": { - "pincode": 400004 - } - } - } - }, - { - "item": "Shampoo", - "props": { - "seller": { - "address": { - "pincode": 400004 - } - } - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json deleted file mode 100644 index 4db8824c..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_neq_filter_response.json +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - "item": "Shampoo", - "props": { - "brand": "Sunsilk" - } - }, - { - "item": "Soap", - "props": { - "brand": "Lifebuoy" - } - } -] diff --git a/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json b/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json deleted file mode 100644 index 7b3e64bb..00000000 --- a/document-store/src/integrationTest/resources/query/flat_json_not_contains_filter_response.json +++ /dev/null @@ -1,37 +0,0 @@ -[ - { - "item": "Mirror" - }, - { - "item": "Shampoo", - "props": { - "colors": [ - "Black" - ] - } - }, - { - "item": "Shampoo" - }, - { - "item": "Soap", - "props": { - "colors": [ - "Orange", - "Blue" - ] - } - }, - { - "item": "Comb" - }, - { - "item": "Comb", - "props": { - "colors": [] - } - }, - { - "item": "Soap" - } -] diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java index 370c724d..6d3abf4f 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java @@ -68,19 +68,6 @@ public T accept(final FieldTransformationVisitor visitor) { return visitor.visit(this); } - /** - * Accepts a select type expression visitor. Overrides the parent to dispatch to the - * JsonIdentifierExpression-specific visit method. - * - * @param visitor The select type expression visitor - * @param The return type - * @return The result of visiting this expression - */ - @Override - public T accept(final org.hypertrace.core.documentstore.parser.SelectTypeExpressionVisitor visitor) { - return visitor.visit(this); - } - @Override public String toString() { return String.format( diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java index bf0b7375..a6e0ed0e 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/parser/SelectTypeExpressionVisitor.java @@ -6,7 +6,6 @@ import org.hypertrace.core.documentstore.expression.impl.ConstantExpression.DocumentConstantExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; public interface SelectTypeExpressionVisitor { T visit(final AggregateExpression expression); @@ -20,8 +19,4 @@ public interface SelectTypeExpressionVisitor { T visit(final IdentifierExpression expression); T visit(final AliasedIdentifierExpression expression); - - default T visit(final JsonIdentifierExpression expression) { - return visit((IdentifierExpression) expression); - } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java index 2bce8e04..d2c444a9 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java @@ -1,7 +1,6 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; @@ -17,25 +16,16 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - // Check if LHS is a JSON field (JSONB column access) - boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - - // Check if the collection type is flat or nested - boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - - // Use JSON parser for: - // 1. Nested collections - !isFlatCollection - // 2. JSON fields within flat collections - isJsonField - boolean useJsonParser = !isFlatCollection || isJsonField; - - if (useJsonParser) { - // Use the JSON logic for JSON document fields - jsonContainsParser.parse(expression, context); // This adds the parameter. - return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); - } else { - // Use the non-JSON logic for first-class array fields + boolean isFirstClassField = + context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + if (isFirstClassField) { + // Use the non-JSON logic for first-class fields String containsExpression = nonJsonContainsParser.parse(expression, context); return String.format("%s IS NULL OR NOT (%s)", parsedLhs, containsExpression); + } else { + // Use the JSON logic for document fields. + jsonContainsParser.parse(expression, context); // This adds the parameter. + return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); } } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java index ce204c5b..91c80986 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java @@ -1,7 +1,6 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; @@ -17,26 +16,17 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - PostgresInRelationalFilterParserInterface inFilterParser = - getInFilterParser(expression, context); + PostgresInRelationalFilterParserInterface inFilterParser = getInFilterParser(context); final String parsedInExpression = inFilterParser.parse(expression, context); return String.format("%s IS NULL OR NOT (%s)", parsedLhs, parsedInExpression); } private PostgresInRelationalFilterParserInterface getInFilterParser( - final RelationalExpression expression, PostgresRelationalFilterContext context) { - // Check if LHS is a JSON field (JSONB column access) - boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + PostgresRelationalFilterContext context) { + boolean isFirstClassField = + context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - // Check if the collection type is flat or nested - boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - - // Use JSON parser for: - // 1. Nested collections - !isFlatCollection - // 2. JSON fields within flat collections - isJsonField - boolean useJsonParser = !isFlatCollection || isJsonField; - - return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; + return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java index 725d3aa2..0fd575b8 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java @@ -12,17 +12,15 @@ import com.google.common.collect.Maps; import java.util.Map; -import org.hypertrace.core.documentstore.DocumentType; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.expression.operators.RelationalOperator; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; +import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FlatPostgresFieldTransformer; public class PostgresRelationalFilterParserFactoryImpl implements PostgresRelationalFilterParserFactory { - private static final Map parserMap = Maps.immutableEnumMap( Map.ofEntries( @@ -54,39 +52,15 @@ public class PostgresRelationalFilterParserFactoryImpl public PostgresRelationalFilterParser parser( final RelationalExpression expression, final PostgresQueryParser postgresQueryParser) { - // Determine if we should use JSON-specific parsers based on: - // 1. Collection type (nested collections store entire document as JSONB) - // 2. Field type (flat collections may have JSONB columns for nested data) - boolean useJsonParser = shouldUseJsonParser(expression, postgresQueryParser); + boolean isFirstClassField = + postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; if (expression.getOperator() == CONTAINS) { - return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; + return isFirstClassField ? nonJsonFieldContainsParser : jsonFieldContainsParser; } else if (expression.getOperator() == IN) { - return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; + return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; } return parserMap.getOrDefault(expression.getOperator(), postgresStandardRelationalFilterParser); } - - /** - * Determines if JSON-specific parser should be used based on collection type and field type. - * - * @param expression the relational expression to evaluate - * @param postgresQueryParser the query parser context - * @return true if JSON parser should be used, false for standard SQL parser - */ - private boolean shouldUseJsonParser( - final RelationalExpression expression, final PostgresQueryParser postgresQueryParser) { - // Check if the collection type is flat or nested - boolean isFlatCollection = - postgresQueryParser.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - - // Check if LHS is a JSON field (JSONB column access) - boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - - // Use JSON parsers for: - // 1. Nested collections (entire document is JSONB) - !isFlatCollection - // 2. JSON fields within flat collections - isJsonField - return !isFlatCollection || isJsonField; - } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java index aace4e97..7052b90a 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresSelectionQueryTransformer.java @@ -10,7 +10,6 @@ import org.hypertrace.core.documentstore.expression.impl.ConstantExpression.DocumentConstantExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.type.GroupTypeExpression; import org.hypertrace.core.documentstore.expression.type.SelectTypeExpression; import org.hypertrace.core.documentstore.parser.GroupTypeExpressionVisitor; @@ -112,11 +111,6 @@ public Boolean visit(IdentifierExpression expression) { return false; } - @Override - public Boolean visit(JsonIdentifierExpression expression) { - return false; - } - @Override public Boolean visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); @@ -150,11 +144,6 @@ public Boolean visit(IdentifierExpression expression) { return true; } - @Override - public Boolean visit(JsonIdentifierExpression expression) { - return true; - } - @Override public Boolean visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java index 75876d0d..8145b8f7 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/transformer/PostgresUnnestQueryTransformer.java @@ -16,7 +16,6 @@ import org.hypertrace.core.documentstore.expression.impl.DocumentArrayFilterExpression; import org.hypertrace.core.documentstore.expression.impl.FunctionExpression; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.KeyExpression; import org.hypertrace.core.documentstore.expression.impl.LogicalExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; @@ -220,11 +219,6 @@ public List visit(IdentifierExpression expression) { return List.of(expression.getName()); } - @Override - public List visit(JsonIdentifierExpression expression) { - return List.of(expression.getName()); - } - @Override public List visit(AliasedIdentifierExpression expression) { throw new UnsupportedOperationException("This operation is not supported"); diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java index a3f319d7..44602b45 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java @@ -2,7 +2,6 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FieldToPgColumn; import org.hypertrace.core.documentstore.postgres.utils.PostgresUtils.Type; @@ -49,15 +48,4 @@ public String visit(final IdentifierExpression expression) { .getPgColTransformer() .buildFieldAccessorWithCast(fieldToPgColumn, this.type); } - - @Override - public String visit(final JsonIdentifierExpression expression) { - FieldToPgColumn fieldToPgColumn = getPostgresQueryParser().transformField(expression); - - // Type parameter is ignored for JSON fields - always returns JSONB - // buildFieldAccessorWithCast will use -> for all accessors without casting - return getPostgresQueryParser() - .getPgColTransformer() - .buildFieldAccessorWithCast(fieldToPgColumn, this.type); - } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java index 77f9fe4f..189361ba 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFieldIdentifierExpressionVisitor.java @@ -2,7 +2,6 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FieldToPgColumn; @@ -29,12 +28,4 @@ public String visit(final IdentifierExpression expression) { .getPgColTransformer() .buildFieldAccessorWithoutCast(fieldToPgColumn); } - - @Override - public String visit(final JsonIdentifierExpression expression) { - FieldToPgColumn fieldToPgColumn = getPostgresQueryParser().transformField(expression); - return getPostgresQueryParser() - .getPgColTransformer() - .buildFieldAccessorWithoutCast(fieldToPgColumn); - } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java index 8402f5c7..1fe4ae1d 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresIdentifierExpressionVisitor.java @@ -2,7 +2,6 @@ import lombok.NoArgsConstructor; import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; -import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; @NoArgsConstructor @@ -25,9 +24,4 @@ public PostgresQueryParser getPostgresQueryParser() { public String visit(final IdentifierExpression expression) { return expression.getName(); } - - @Override - public String visit(final JsonIdentifierExpression expression) { - return expression.getName(); - } } diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java index cc963152..fe827257 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionTest.java @@ -1,7 +1,6 @@ package org.hypertrace.core.documentstore.expression.impl; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -126,38 +125,4 @@ void testOfVarargsWithNull() { assertEquals("JSON path cannot be null or empty", exception.getMessage()); } - - /** - * Tests instanceof check - JsonIdentifierExpression should be identifiable. - */ - @Test - void testInstanceOfJsonIdentifierExpression() { - JsonIdentifierExpression jsonExpr = JsonIdentifierExpression.of("props", "brand"); - - // Should be identifiable as JsonIdentifierExpression - assertTrue(jsonExpr instanceof JsonIdentifierExpression, "Should be instanceof JsonIdentifierExpression"); - } - - /** - * Tests instanceof check through base class reference. - */ - @Test - void testInstanceOfThroughBaseReference() { - // Create JsonIdentifierExpression but hold as IdentifierExpression reference - IdentifierExpression expr = JsonIdentifierExpression.of("props", "brand"); - - // Should still work with instanceof check - assertTrue(expr instanceof JsonIdentifierExpression, "instanceof should work through base class reference"); - } - - /** - * Tests that regular IdentifierExpression is not JsonIdentifierExpression. - */ - @Test - void testRegularIdentifierIsNotJsonIdentifier() { - IdentifierExpression expr = IdentifierExpression.of("regularColumn"); - - // Regular IdentifierExpression should not be JsonIdentifierExpression - assertFalse(expr instanceof JsonIdentifierExpression, "Regular identifier should not be instanceof JsonIdentifierExpression"); - } } diff --git a/settings-gradle.lockfile b/settings-gradle.lockfile deleted file mode 100644 index 709a43f7..00000000 --- a/settings-gradle.lockfile +++ /dev/null @@ -1,4 +0,0 @@ -# This is a Gradle generated file for dependency locking. -# Manual edits can break the build and are not advised. -# This file is expected to be part of source control. -empty=incomingCatalogForLibs0 From f9e1f43de16c50b99aa7385cc6c97dce324b2250 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Wed, 22 Oct 2025 12:27:16 +0530 Subject: [PATCH 3/5] Support for flat collection jsonb relational operators --- .../documentstore/DocStoreQueryV1Test.java | 214 ++++++++++++++++++ ...gresNotContainsRelationalFilterParser.java | 25 +- .../PostgresNotInRelationalFilterParser.java | 20 +- ...gresRelationalFilterParserFactoryImpl.java | 17 +- 4 files changed, 259 insertions(+), 17 deletions(-) diff --git a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java index cb192c4b..7520f22a 100644 --- a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java +++ b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java @@ -3981,6 +3981,220 @@ void testFlatVsNestedCollectionNestedFieldSelections(String dataStoreName) throw assertDocsAndSizeEqual( dataStoreName, flatBrandNoAliasIterator, "query/no_alias_response.json", 8); } + + /** + * Tests for relational operators on JSONB nested fields in flat collections. + * Tests: CONTAINS, NOT_CONTAINS, IN, NOT_IN, EQ, NEQ, LT, GT on JSONB columns. + */ + @Nested + class FlatCollectionJsonbRelationalOperatorTest { + + /** + * Tests CONTAINS and NOT_CONTAINS operators on JSONB array fields. - CONTAINS: finds + * documents where array contains the value - NOT_CONTAINS: finds documents where array + * doesn't contain the value (including NULL) + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testJsonbArrayContainsOperators(String dataStoreName) { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Test 1: CONTAINS - props.colors CONTAINS "Green" + // Expected: 1 document (id=1, Dettol Soap has ["Green", "White"]) + Query containsQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "colors"), + CONTAINS, + ConstantExpression.of("Green"))) + .build(); + + long containsCount = flatCollection.count(containsQuery); + assertEquals(1, containsCount, "CONTAINS: Should find 1 document with Green color"); + + // Test 2: NOT_CONTAINS - props.colors NOT_CONTAINS "Green" AND _id <= 8 + // Expected: 7 documents (all except id=1 which has Green, limited to first 8) + Query notContainsQuery = + Query.builder() + .setFilter( + LogicalExpression.builder() + .operator(LogicalOperator.AND) + .operand( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "colors"), + NOT_CONTAINS, + ConstantExpression.of("Green"))) + .operand( + RelationalExpression.of( + IdentifierExpression.of("_id"), LTE, ConstantExpression.of(8))) + .build()) + .build(); + + long notContainsCount = flatCollection.count(notContainsQuery); + assertEquals( + 7, notContainsCount, "NOT_CONTAINS: Should find 7 documents without Green color"); + } + + /** + * Tests IN and NOT_IN operators on JSONB scalar fields. - IN: finds documents where field + * value is in the provided list - NOT_IN: finds documents where field value is not in the + * list (including NULL) + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testJsonbScalarInOperators(String dataStoreName) { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Test 1: IN - props.brand IN ["Dettol", "Lifebuoy"] + // Expected: 2 documents (id=1 Dettol, id=5 Lifebuoy) + Query inQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + IN, + ConstantExpression.ofStrings(List.of("Dettol", "Lifebuoy")))) + .build(); + + long inCount = flatCollection.count(inQuery); + assertEquals(2, inCount, "IN: Should find 2 documents with Dettol or Lifebuoy brand"); + + // Test 2: NOT_IN - props.brand NOT_IN ["Dettol"] AND _id <= 8 + // Expected: 7 documents (all except id=1 which is Dettol, limited to first 8) + Query notInQuery = + Query.builder() + .setFilter( + LogicalExpression.builder() + .operator(LogicalOperator.AND) + .operand( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + NOT_IN, + ConstantExpression.ofStrings(List.of("Dettol")))) + .operand( + RelationalExpression.of( + IdentifierExpression.of("_id"), LTE, ConstantExpression.of(8))) + .build()) + .build(); + + long notInCount = flatCollection.count(notInQuery); + assertEquals(7, notInCount, "NOT_IN: Should find 7 documents without Dettol brand"); + } + + /** + * Tests EQ and NEQ operators on JSONB scalar fields. - EQ: finds documents where field equals + * the value - NEQ: finds documents where field doesn't equal the value (excluding NULL) + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testJsonbScalarEqualityOperators(String dataStoreName) { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Test 1: EQ - props.brand EQ "Dettol" + // Expected: 1 document (id=1, Dettol Soap) + Query eqQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + EQ, + ConstantExpression.of("Dettol"))) + .build(); + + long eqCount = flatCollection.count(eqQuery); + assertEquals(1, eqCount, "EQ: Should find 1 document with Dettol brand"); + + // Test 2: NEQ - props.brand NEQ "Dettol" (no _id filter needed) + // Expected: 2 documents (id=3 Sunsilk, id=5 Lifebuoy, excluding NULL props) + Query neqQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "brand"), + NEQ, + ConstantExpression.of("Dettol"))) + .build(); + + long neqCount = flatCollection.count(neqQuery); + assertEquals(2, neqCount, "NEQ: Should find 2 documents without Dettol brand"); + } + + /** + * Tests LT, GT, LTE, GTE comparison operators on JSONB numeric fields. Tests deeply nested + * numeric fields like props.seller.address.pincode. Data: ids 1,3 have pincode 400004; ids + * 5,7 have pincode 700007; rest are NULL + */ + @ParameterizedTest + @ArgumentsSource(PostgresProvider.class) + void testJsonbNumericComparisonOperators(String dataStoreName) { + Datastore datastore = datastoreMap.get(dataStoreName); + Collection flatCollection = + datastore.getCollectionForType(FLAT_COLLECTION_NAME, DocumentType.FLAT); + + // Test 1: GT - props.seller.address.pincode > 500000 + // Expected: 2 documents (ids 5,7 with pincode 700007 in Kolkata) + Query gtQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + GT, + ConstantExpression.of(500000))) + .build(); + + long gtCount = flatCollection.count(gtQuery); + assertEquals(2, gtCount, "GT: Should find 2 documents with pincode > 500000"); + + // Test 2: LT - props.seller.address.pincode < 500000 + // Expected: 2 documents (ids 1,3 with pincode 400004 in Mumbai) + Query ltQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + LT, + ConstantExpression.of(500000))) + .build(); + + long ltCount = flatCollection.count(ltQuery); + assertEquals(2, ltCount, "LT: Should find 2 documents with pincode < 500000"); + + // Test 3: GTE - props.seller.address.pincode >= 700000 + // Expected: 2 documents (ids 5,7 with pincode 700007) + Query gteQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + GTE, + ConstantExpression.of(700000))) + .build(); + + long gteCount = flatCollection.count(gteQuery); + assertEquals(2, gteCount, "GTE: Should find 2 documents with pincode >= 700000"); + + // Test 4: LTE - props.seller.address.pincode <= 400004 + // Expected: 2 documents (ids 1,3 with pincode 400004) + Query lteQuery = + Query.builder() + .setFilter( + RelationalExpression.of( + JsonIdentifierExpression.of("props", "seller", "address", "pincode"), + LTE, + ConstantExpression.of(400004))) + .build(); + + long lteCount = flatCollection.count(lteQuery); + assertEquals(2, lteCount, "LTE: Should find 2 documents with pincode <= 400004"); + } + } } @Nested diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java index d2c444a9..ace7960f 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotContainsRelationalFilterParser.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; @@ -16,16 +17,26 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - boolean isFirstClassField = - context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; - if (isFirstClassField) { + boolean useJsonParser = shouldUseJsonParser(expression, context); + + if (useJsonParser) { + // Use the JSON logic for JSON document fields + jsonContainsParser.parse(expression, context); // This adds the parameter. + return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); + } else { // Use the non-JSON logic for first-class fields String containsExpression = nonJsonContainsParser.parse(expression, context); return String.format("%s IS NULL OR NOT (%s)", parsedLhs, containsExpression); - } else { - // Use the JSON logic for document fields. - jsonContainsParser.parse(expression, context); // This adds the parameter. - return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); } } + + private boolean shouldUseJsonParser( + final RelationalExpression expression, final PostgresRelationalFilterContext context) { + + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + boolean useJsonParser = !isFlatCollection || isJsonField; + + return useJsonParser; + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java index 91c80986..ce204c5b 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotInRelationalFilterParser.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; import org.hypertrace.core.documentstore.DocumentType; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; @@ -16,17 +17,26 @@ public String parse( final RelationalExpression expression, final PostgresRelationalFilterContext context) { final String parsedLhs = expression.getLhs().accept(context.lhsParser()); - PostgresInRelationalFilterParserInterface inFilterParser = getInFilterParser(context); + PostgresInRelationalFilterParserInterface inFilterParser = + getInFilterParser(expression, context); final String parsedInExpression = inFilterParser.parse(expression, context); return String.format("%s IS NULL OR NOT (%s)", parsedLhs, parsedInExpression); } private PostgresInRelationalFilterParserInterface getInFilterParser( - PostgresRelationalFilterContext context) { - boolean isFirstClassField = - context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + final RelationalExpression expression, PostgresRelationalFilterContext context) { + // Check if LHS is a JSON field (JSONB column access) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; - return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; + // Check if the collection type is flat or nested + boolean isFlatCollection = context.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + // Use JSON parser for: + // 1. Nested collections - !isFlatCollection + // 2. JSON fields within flat collections - isJsonField + boolean useJsonParser = !isFlatCollection || isJsonField; + + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java index 0fd575b8..7df0d94a 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresRelationalFilterParserFactoryImpl.java @@ -12,12 +12,13 @@ import com.google.common.collect.Maps; import java.util.Map; +import org.hypertrace.core.documentstore.DocumentType; +import org.hypertrace.core.documentstore.expression.impl.JsonIdentifierExpression; import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; import org.hypertrace.core.documentstore.expression.operators.RelationalOperator; import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresContainsRelationalFilterParserNonJsonField; import org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field.PostgresInRelationalFilterParserNonJsonField; -import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FlatPostgresFieldTransformer; public class PostgresRelationalFilterParserFactoryImpl implements PostgresRelationalFilterParserFactory { @@ -52,13 +53,19 @@ public class PostgresRelationalFilterParserFactoryImpl public PostgresRelationalFilterParser parser( final RelationalExpression expression, final PostgresQueryParser postgresQueryParser) { - boolean isFirstClassField = - postgresQueryParser.getPgColTransformer() instanceof FlatPostgresFieldTransformer; + // Check if LHS is a JSON field (JSONB column access) + boolean isJsonField = expression.getLhs() instanceof JsonIdentifierExpression; + + // Check if the collection type is flat or nested + boolean isFlatCollection = + postgresQueryParser.getPgColTransformer().getDocumentType() == DocumentType.FLAT; + + boolean useJsonParser = !isFlatCollection || isJsonField; if (expression.getOperator() == CONTAINS) { - return isFirstClassField ? nonJsonFieldContainsParser : jsonFieldContainsParser; + return useJsonParser ? jsonFieldContainsParser : nonJsonFieldContainsParser; } else if (expression.getOperator() == IN) { - return isFirstClassField ? nonJsonFieldInFilterParser : jsonFieldInFilterParser; + return useJsonParser ? jsonFieldInFilterParser : nonJsonFieldInFilterParser; } return parserMap.getOrDefault(expression.getOperator(), postgresStandardRelationalFilterParser); From f115bc35f4764c7a8392111d5c951c5d13013ff7 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Wed, 22 Oct 2025 12:41:15 +0530 Subject: [PATCH 4/5] Spotless --- .../hypertrace/core/documentstore/DocStoreQueryV1Test.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java index 7520f22a..e9c53571 100644 --- a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java +++ b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java @@ -3983,8 +3983,8 @@ void testFlatVsNestedCollectionNestedFieldSelections(String dataStoreName) throw } /** - * Tests for relational operators on JSONB nested fields in flat collections. - * Tests: CONTAINS, NOT_CONTAINS, IN, NOT_IN, EQ, NEQ, LT, GT on JSONB columns. + * Tests for relational operators on JSONB nested fields in flat collections. Tests: CONTAINS, + * NOT_CONTAINS, IN, NOT_IN, EQ, NEQ, LT, GT on JSONB columns. */ @Nested class FlatCollectionJsonbRelationalOperatorTest { From b64ad8c6da6953cdd665f0d315555a5ccd8c2574 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Wed, 22 Oct 2025 23:18:01 +0530 Subject: [PATCH 5/5] Add more test cases to PostgresQueryParserTest.java --- .../query/v1/PostgresQueryParserTest.java | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParserTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParserTest.java index 282ff47a..67b0e63b 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParserTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParserTest.java @@ -21,6 +21,7 @@ import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LTE; import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NEQ; import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NOT_CONTAINS; +import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NOT_IN; import static org.hypertrace.core.documentstore.expression.operators.SortOrder.ASC; import static org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -1427,4 +1428,127 @@ void testCollectionInOtherSchema() { assertEquals( "SELECT * FROM test_schema.\"test_table.with_a_dot\"", postgresQueryParser.parse()); } + + @Test + void testNotContainsWithFlatCollectionNonJsonField() { + Query query = + Query.builder() + .setFilter( + RelationalExpression.of( + IdentifierExpression.of("tags"), + NOT_CONTAINS, + ConstantExpression.of("premium"))) + .build(); + + PostgresQueryParser postgresQueryParser = + new PostgresQueryParser( + TEST_TABLE, + PostgresQueryTransformer.transform(query), + new FlatPostgresFieldTransformer()); + + String sql = postgresQueryParser.parse(); + assertEquals( + "SELECT * FROM \"testCollection\" WHERE \"tags\" IS NULL OR NOT (\"tags\" @> ARRAY[?]::text[])", + sql); + + Params params = postgresQueryParser.getParamsBuilder().build(); + assertEquals(1, params.getObjectParams().size()); + assertEquals(List.of("premium"), params.getObjectParams().get(1)); + } + + @Test + void testNotContainsWithNestedCollectionJsonField() { + Query query = + Query.builder() + .setFilter( + RelationalExpression.of( + IdentifierExpression.of("attributes"), + NOT_CONTAINS, + ConstantExpression.of("value1"))) + .build(); + + PostgresQueryParser postgresQueryParser = + new PostgresQueryParser(TEST_TABLE, PostgresQueryTransformer.transform(query)); + + String sql = postgresQueryParser.parse(); + assertEquals( + "SELECT * FROM \"testCollection\" WHERE document->'attributes' IS NULL OR NOT document->'attributes' @> ?::jsonb", + sql); + + Params params = postgresQueryParser.getParamsBuilder().build(); + assertEquals(1, params.getObjectParams().size()); + assertEquals("[\"value1\"]", params.getObjectParams().get(1)); + } + + @Test + void testNotInWithFlatCollectionNonJsonField() { + Query query = + Query.builder() + .setFilter( + RelationalExpression.of( + IdentifierExpression.of("category"), + NOT_IN, + ConstantExpression.ofStrings(List.of("electronics", "clothing")))) + .build(); + + PostgresQueryParser postgresQueryParser = + new PostgresQueryParser( + TEST_TABLE, + PostgresQueryTransformer.transform(query), + new FlatPostgresFieldTransformer()); + + String sql = postgresQueryParser.parse(); + assertEquals( + "SELECT * FROM \"testCollection\" WHERE \"category\" IS NULL OR NOT (ARRAY[\"category\"]::text[] && ARRAY[?, ?]::text[])", + sql); + + Params params = postgresQueryParser.getParamsBuilder().build(); + assertEquals(2, params.getObjectParams().size()); + } + + @Test + void testNotInWithNestedCollectionJsonField() { + Query query = + Query.builder() + .setFilter( + RelationalExpression.of( + IdentifierExpression.of("status"), + NOT_IN, + ConstantExpression.ofStrings(List.of("active", "pending")))) + .build(); + + PostgresQueryParser postgresQueryParser = + new PostgresQueryParser(TEST_TABLE, PostgresQueryTransformer.transform(query)); + + String sql = postgresQueryParser.parse(); + assertEquals( + "SELECT * FROM \"testCollection\" WHERE document->'status' IS NULL OR NOT ((((jsonb_typeof(to_jsonb(document->'status')) = 'array' AND to_jsonb(document->'status') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'status') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'status')) = 'array' AND to_jsonb(document->'status') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'status') @> jsonb_build_array(?)))))", + sql); + + Params params = postgresQueryParser.getParamsBuilder().build(); + assertEquals(4, params.getObjectParams().size()); + } + + @Test + void testContainsWithFlatCollectionNonJsonField() { + Query query = + Query.builder() + .setFilter( + RelationalExpression.of( + IdentifierExpression.of("keywords"), CONTAINS, ConstantExpression.of("java"))) + .build(); + + PostgresQueryParser postgresQueryParser = + new PostgresQueryParser( + TEST_TABLE, + PostgresQueryTransformer.transform(query), + new FlatPostgresFieldTransformer()); + + String sql = postgresQueryParser.parse(); + assertEquals("SELECT * FROM \"testCollection\" WHERE \"keywords\" @> ARRAY[?]::text[]", sql); + + Params params = postgresQueryParser.getParamsBuilder().build(); + assertEquals(1, params.getObjectParams().size()); + assertEquals(List.of("java"), params.getObjectParams().get(1)); + } }