Skip to content

Conversation

@tobyhede
Copy link
Contributor

@tobyhede tobyhede commented Dec 11, 2025

Adds support for transforming JSONB containment operators (@> and <@) to EQL function calls, enabling GIN index usage with encrypted JSONB columns.

Key changes:

  • Add RewriteContainmentOps transformation rule that converts @> to eql_v2.jsonb_contains() and <@ to eql_v2.jsonb_contained_by()
  • Only rewrite when at least one operand is EQL-typed (preserves native JSONB operations)
  • Add EXPLAIN statement support for type checking
  • Comprehensive integration test coverage for all operand type combinations

Test Coverage Matrix

Test Pattern Protocol Result
encrypted_contains_param e @> $1 Extended 50 rows
encrypted_contains_literal e @> 'json' Simple 50 rows
param_contains_encrypted $1 @> e Extended 1 row (exact match)
literal_contains_encrypted 'json' @> e Simple 1 row (exact match)
jsonb_contains_function_works eql_v2.jsonb_contains() Extended 50 rows

Test plan

  • Unit tests for containment operator transformation
  • Integration tests for all operand combinations (param/literal × LHS/RHS)
  • Tests pass in parallel execution
  • Full integration test suite

@tobyhede tobyhede changed the title WIP proxy converts jsonb containment to eql function call feat(eql-mapper): JSONB containment operator transformation (@> and <@) Dec 12, 2025
Copy link
Contributor

@freshtonic freshtonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Very Good. Testing looks thorough too.

I'm glad that the bus factor on EQL Mapper is > 1.

@tobyhede
Copy link
Contributor Author

NOTE: build failing because proxy needs to use the new version of EQL. Will fix once that is merged.

@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch from cf15b60 to 144d8fe Compare December 14, 2025 01:30
…sure

The identity closure |t| t in map_or_else is unnecessary.
Using unwrap_or_else is more idiomatic when the Some value
doesn't need transformation.
@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch 5 times, most recently from 8e21923 to 3f7b5e8 Compare December 15, 2025 01:09
- Use mem::replace instead of clone() to preserve NodeKey identity
  for downstream casting rules in RewriteContainmentOps
- Move RewriteContainmentOps before CastLiteralsAsEncrypted in the
  transformation pipeline so literals are properly encrypted after
  operator rewriting
- Add Statement::Explain to requires_type_check() so EXPLAIN queries
  are processed through the type inference pipeline
- Handle EXPLAIN in statement type inference with empty projection
… operator

Work in progress integration tests for JSONB containment operations.
Tests use direct @> operator to verify GIN index usage with encrypted columns.
RewriteContainmentOps was unconditionally rewriting all @> and <@
operators without verifying that operands were EQL-typed. This caused
issues with native JSONB operations.

Key changes:
- Added uses_eql_type() and is_eql_typed() methods to check node_types
- would_edit() now uses node_path.last_1_as() to get original AST nodes
  with correct NodeKey identity (matching type inference)
- Only rewrites when at least one operand is EQL-typed
- Removed #[allow(dead_code)] since node_types is now used

The fix uses node_path instead of target_node because the transformation
framework clones nodes, causing different memory addresses. The node_path
preserves the original AST node references used during type inference.
…tests

- Use dedicated ID range (1000000-1000499) for fixture data
- Remove clear() call to avoid data race with parallel tests
- Add ensure_fixture_data() that inserts with ON CONFLICT DO NOTHING
- Filter queries by fixture ID range for test isolation
- All 5 containment tests now pass in parallel execution

Coverage matrix:
- encrypted_contains_param: e @> $1 (extended protocol)
- encrypted_contains_literal: e @> 'json' (simple protocol)
- param_contains_encrypted: $1 @> e (extended protocol)
- literal_contains_encrypted: 'json' @> e (simple protocol)
- jsonb_contains_function_works: eql_v2.jsonb_contains()
For param_contains_encrypted and literal_contains_encrypted tests:
- Use exact match search value {"string": "value_1", "number": 1}
- This matches exactly 1 row (where the search value equals the stored value)
- Previously these tests returned 0 rows using subset search

Search value now varies by test type:
- column @> search: subset {"string": "value_1"} matches ~50 rows
- search @> column: exact {"string": "value_1", "number": 1} matches 1 row
- Add #[inline] hint to uses_eql_type method
- Fix unused variable naming convention (_inner_statement)
- Replace magic number 500 with FIXTURE_COUNT constant
- Add comments explaining variance tolerances for test assertions
- Use matches! macro instead of match expression returning bool
- Replace format!() with .to_string() for static strings
- Use unwrap_or_else instead of map_or_else with identity closure
Use (40..=60).contains(&count) instead of count >= 40 && count <= 60
Move comment and underscore prefix into pattern destructuring rather
than using a separate let binding to suppress the unused variable warning.
Simplify integration tests by removing unnecessary ::jsonb type casts
from INSERT and SELECT statements. The test macros no longer require
the $cast parameter since jsonb columns don't need explicit casting.
Add test that detects silent encryption failures by:
- Inserting via proxy (should encrypt)
- Querying directly from PostgreSQL (bypassing proxy)
- Verifying stored value differs from plaintext
- Round-trip verification through proxy

Includes query_direct helpers in common.rs for direct database access.
Add notes to searchable-json.md clarifying that JSONB literals in INSERT,
UPDATE, and containment operations work directly without ::jsonb type
annotations. The proxy infers the JSONB type from the target column.

Also adds docstring to map_jsonb integration test explaining the
intentional absence of type casts.
@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch from 3f7b5e8 to 0a33b74 Compare December 15, 2025 01:12
@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch from 686eeb1 to 393bbc2 Compare December 15, 2025 01:24
@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch 3 times, most recently from 2386818 to 2430fe6 Compare December 15, 2025 02:19
@tobyhede tobyhede force-pushed the feature/gin-jsonb-containment-tests branch from 2430fe6 to 6d5d45c Compare December 15, 2025 02:27
@tobyhede tobyhede merged commit 88c5450 into main Dec 15, 2025
5 checks passed
@tobyhede tobyhede deleted the feature/gin-jsonb-containment-tests branch December 15, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants