-
Notifications
You must be signed in to change notification settings - Fork 1
feat(eql-mapper): JSONB containment operator transformation (@> and <@) #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
freshtonic
approved these changes
Dec 12, 2025
Contributor
freshtonic
left a comment
There was a problem hiding this 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.
packages/eql-mapper/src/inference/infer_type_impls/statement.rs
Outdated
Show resolved
Hide resolved
tobyhede
commented
Dec 13, 2025
Contributor
Author
|
NOTE: build failing because proxy needs to use the new version of EQL. Will fix once that is merged. |
coderdan
approved these changes
Dec 13, 2025
cf15b60 to
144d8fe
Compare
…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.
8e21923 to
3f7b5e8
Compare
… function declarations
…ntained_by functions
- 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.
3f7b5e8 to
0a33b74
Compare
686eeb1 to
393bbc2
Compare
2386818 to
2430fe6
Compare
2430fe6 to
6d5d45c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for transforming JSONB containment operators (@> and <@) to EQL function calls, enabling GIN index usage with encrypted JSONB columns.
Key changes:
Test Coverage Matrix
Test plan