perf: flip eql_v2_encrypted infix operator implementations to inlinable SQL (#193)#196
Open
perf: flip eql_v2_encrypted infix operator implementations to inlinable SQL (#193)#196
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR centralizes root-level equality/hashing on HMAC-256, rewrites many operator wrappers as inlinable SQL (IMMUTABLE/STRICT/PARALLEL SAFE), updates ste_vec element matching to use eql_v2.eq, and adjusts pinning/CI/test fixtures and docs to reflect the HMAC-only contract. ChangesPhase 1 HMAC-256 Root Constraint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderdan
force-pushed
the
dan/phase-1-operator-inlining
branch
2 times, most recently
from
c80c760 to
1ab0b00
Compare
Contributor
Author
|
Pushed The 11 operator overloads this PR makes inlinable — 3×
Verified the 11 functions retain I haven't touched the existing test failures ( Filed #198 to track the broader story of why downstream Supabase users will still see these flagged in their Security Advisor until EQL can ship as a real |
coderdan
force-pushed
the
dan/phase-1-operator-inlining
branch
from
1ab0b00 to
a5229a6
Compare
…le SQL (#193) Resolves cipherstash/stack#420 (encryptedSupabase silent seq-scan) by making the `=`, `<>`, `~~`, `~~*`, `@>`, and `<@` operator implementations on `eql_v2_encrypted` eligible for planner inlining. The Postgres planner inlines a custom operator's implementation function during index matching, provided the function is `LANGUAGE sql IMMUTABLE` with a single-statement body and no `SET` clause. Previously every operator wrapper was either `LANGUAGE plpgsql` (which can never be inlined) or had `SET search_path = pg_catalog, extensions, public` (which blocks inlining even on sql functions). As a result, bare queries like `WHERE col = val` from PostgREST and ORMs that don't wrap columns themselves silently fell back to seq scan on every install where the catch-all `eql_v2.encrypted_operator_class` btree wasn't available — most prominently Supabase. This change rewrites the wrappers as inlinable SQL, so the planner reduces them during planning and matches functional indexes built on the underlying extractors: WHERE col = val inlines to eql_v2.hmac_256(col) = eql_v2.hmac_256(val) └── matches functional hash idx WHERE col ~~ val inlines to eql_v2.bloom_filter(col) @> eql_v2.bloom_filter(val) └── matches functional GIN idx WHERE col @> val inlines to eql_v2.ste_vec_contains(col, val) (preserves) Verified empirically: bare `WHERE enc = ...` produces a Bitmap Index Scan on the hmac functional index, where it previously seq-scanned. The lint introduced in the parent commit (#194) goes from 12+ inlinability errors on these operators to zero. Comparison operators (<, <=, >, >=), JSONB extractors (->, ->>), and ORE block operators remain flagged — those are out of scope for Phase 1 and tracked separately in the predicate/extractor RFC. Behaviour change: `=` and `<>` previously dispatched through `eql_v2.compare`, which fell back to ORE / Blake3 / literal comparison when the column's HMAC variant wasn't present. The new implementations require the column to have `equality` configured (i.e. carry an `hm` field). Calling `=` on an ORE-only encrypted column now returns NULL instead of a Boolean — surfacing the config error rather than hiding it. This is intentional per the RFC (`docs/plans/uniform-predicate-extractor-pairs-rfc.md`).
…pose Implements the v2 payload scheme discipline that PR #193 began. Tightens operator-class scaffolding so functional indexes engage uniformly across self-hosted and Supabase, removes root-level Blake3 dead code, and drops test coverage of fictional payload shapes. SQL changes ----------- - `eql_v2.compare`: drop the Blake3 branch from the equality fallback. Equality at the root is hmac-only; ordering branches (ORE, OPE) unchanged. Blake3 is preserved for ste_vec internal element comparison. - `eql_v2.hash_encrypted`: hmac-only. The previous Blake3-first priority existed to satisfy a hash/equality contract that has no production analogue (root payloads never carry b3). Hashing now requires a `unique` index and raises clearly if absent. - `eql_v2.ste_vec_contains`: switch the inner element comparison to `eql_v2.eq` (which routes through compare's literal fallback for ste_vec elements lacking hm/ORE/OPE). Documents why neither bare `=` nor `compare_blake3` is appropriate here. - `tasks/pin_search_path.sql`: extend the inline-critical allowlist to cover the operators rewritten in #193 — `=`, `<>`, `~~`, `~~*` for same-type and both cross-type (encrypted, jsonb) directions. Without this, the post-install search_path pin re-disables inlining on the very wrappers #193 made inlinable. Test changes ------------ - `tests/sqlx/migrations/004_install_test_helpers.sql`: drop the fictional root-level `b3` field from `create_encrypted_json`. ste_vec inner elements still carry b3 via `get_numeric_ste_vec_*`. - `equality_tests`: remove `equality_operator_*_blake3` and `eq_function_*_blake3` (6 + 2 tests). Tested root-level Blake3 fallthrough that no longer exists. - `inequality_tests`: remove `*_finds_non_matching_records_blake3`. - `index_compare_tests`: remove `blake3_compare_{equal,less,greater}`. - `hash_operator_tests`: remove `mixed_index_*` (3 tests, fictional hm+b3 vs b3-only contract), `hash_function_uses_blake3_first`, `hash_consistency_full_index_matches_blake3_only`, `hmac_and_blake3_produce_different_hashes`, `ste_vec_wrapped_hashes_same_as_unwrapped` (Blake3-first hash priority contract). Update remaining error-message assertions to reference `hmac_256` only. - `ope_tests`: remove `encrypted_{eq,neq}_operator_uses_op{f,v}` — OPE-only payloads no longer support `=`/`<>` (no hm at root). Range operators on OPE remain. - `ore_equality_tests`: remove `ore64`/`ore_cllw_*` `eq`/`neq` tests for the same reason. - `ore_text_operator_tests`: remove ORE-text `=`/`<>` variants (same-type and both cross-type directions). - `operator_class_tests`: rewrite the affected EXPLAIN-shape assertions to use hmac literals instead of ob/ore-only literals. - `operator_compare_tests`: remove `compare_blake3_index` (root-level compare via Blake3 no longer exists). - `constraint_tests`: switch `add_encrypted_constraint_prevents_invalid_data` to compare via the underlying jsonb (the `'{}'` literal has no `hm`). See `docs/plans/eql-payload-scheme-discipline-rfc.md` for the full rationale and v3 forward path.
coderdan
force-pushed
the
dan/phase-1-operator-inlining
branch
from
a5229a6 to
8e80a44
Compare
The post-install pin_search_path step intentionally skips =, <>, ~~, @>, <@ and the jsonb_contains/_by wrappers — they must inline for the planner to match the documented functional indexes. Splinter's function_search_path_mutable lint flags those same functions because SET search_path is precisely the thing inlining forbids. Both lists must agree, or splinter blocks CI on the operators we deliberately left unpinned. Adds eql_v2.=, eql_v2.<>, eql_v2.~~ to the splinter allowlist with the same Phase 1 inlining justification used by the inline-critical allowlist in tasks/pin_search_path.sql. Note that the eql_v2.~~* operator routes to the eql_v2.~~ function (case-insensitivity is meaningless on encrypted ciphertexts) so a single allowlist entry covers both LIKE variants. Verified locally: raw=20 findings, allowlisted=20, unallowlisted=0.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/sqlx/tests/ore_equality_tests.rs (1)
1-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winModule docstring is stale — file no longer exercises equality.
The
//!header still describes this as "ORE equality/inequality operator tests" and "Tests equality with different ORE encryption schemes", but every=/<>test is gone (line 10–16 explains why) and only< / <= / > / >=cases remain. The filename is also now misleading; at minimum the doc block should be updated, and you may want to either rename the file (e.g.,ore_comparison_tests.rs) or fold the remaining tests into an existing comparison module to avoid future confusion.📝 Suggested header update
-//! ORE equality/inequality operator tests +//! ORE comparison operator tests //! -//! Tests equality with different ORE encryption schemes (ORE64, CLLW_U64_8, CLLW_VAR_8) +//! Tests `<`, `<=`, `>`, `>=` against different ORE encryption schemes +//! (ORE64, CLLW_U64_8, CLLW_VAR_8). Equality / inequality (`=`, `<>`) require +//! an HMAC root term and are not applicable to ORE-only fixtures. //! Uses ore table from migrations/002_install_ore_data.sql (ids 1-1000)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sqlx/tests/ore_equality_tests.rs` around lines 1 - 5, Update the stale module docstring in ore_equality_tests.rs to describe that the file contains ORE comparison (ordering) tests for <, <=, >, >= across the ORE schemes (ORE64, CLLW_U64_8, CLLW_VAR_8) and uses the ore table from migrations/002_install_ore_data.sql; either rename the file to ore_comparison_tests.rs or move these tests into an existing comparison module to match the new behavior and avoid future confusion.tests/sqlx/tests/index_compare_tests.rs (1)
19-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMacro doc references the now-stale
'b3'example.The
assert_compare!doc block usescreate_encrypted_json(1, 'b3')to motivate theformat!-over-bind decision, butb3is no longer emitted bycreate_encrypted_json, and the tests that exercised that shape were removed (lines 38–43). Recommend updating the example to a still-valid filter such as'hm'so future readers don't follow the stale pattern.📝 Suggested doc tweak
// Helper macro to reduce repetition for compare tests // // Note: Uses format! for SQL construction because test data expressions -// (like "create_encrypted_json(1, 'b3')") must be evaluated by PostgreSQL, +// (like "create_encrypted_json(1, 'hm')") must be evaluated by PostgreSQL, // not passed as parameters. SQLx cannot pass PostgreSQL function calls as // query parameters - they must be part of the SQL string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sqlx/tests/index_compare_tests.rs` around lines 19 - 32, The macro doc for assert_compare! references a stale example create_encrypted_json(1, 'b3'); update that example to use a currently emitted filter value (e.g., replace 'b3' with 'hm') so the rationale for using format! over parameter binding stays correct; locate the macro named assert_compare! and change the quoted literal in its comment from 'b3' to 'hm' (or another valid current example) and run the tests to ensure the comment-only change compiles cleanly.tests/sqlx/tests/inequality_tests.rs (1)
173-189:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale
_blake3jsonb inequality test now duplicates the_hmacvariant.
inequality_operator_encrypted_not_equals_jsonb_blake3builds the operand withcreate_encrypted_json(1)::jsonb - 'ob'and assertscount(2)— identical setup and assertion toinequality_operator_encrypted_not_equals_jsonb_hmacat lines 112–128. Sincecreate_encrypted_jsonno longer emits a rootb3term, this case is no longer Blake3-specific and the name is misleading. Either drop it (consistent with the encrypted/encrypted Blake3 removals immediately above) or rename it to make the distinct intent explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sqlx/tests/inequality_tests.rs` around lines 173 - 189, The test inequality_operator_encrypted_not_equals_jsonb_blake3 duplicates the hmac variant and is no longer Blake3-specific because create_encrypted_json no longer emits a root b3 term; either remove this redundant test or rename it to reflect its generic intent (e.g., inequality_operator_encrypted_not_equals_jsonb) and update any references; locate the test function inequality_operator_encrypted_not_equals_jsonb_blake3 and either delete the entire function or rename it and adjust its test attributes/fixture names accordingly so it no longer claims Blake3-specific behavior.
🧹 Nitpick comments (2)
tests/sqlx/tests/ore_text_operator_tests.rs (1)
13-20: 💤 Low valueEmpty section banners now wrap only removal notes.
The "Equality and inequality operators", "JSONB variants: e = jsonb, e <> jsonb", and "JSONB variants: jsonb = e, jsonb <> e" banners (lines 13–15, 238–240, 246–248) no longer have any tests under them — just the removal comments. Consider either deleting the banners (folding the rationale into the file header alongside the line-3 docstring, which still mentions "equality") or collapsing the three removal notes into a single explanation at the top, so the section structure of the file matches what it actually exercises.
Also applies to: 238-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sqlx/tests/ore_text_operator_tests.rs` around lines 13 - 20, The test file contains empty section headers ("Equality and inequality operators", "JSONB variants: e = jsonb, e <> jsonb", "JSONB variants: jsonb = e, jsonb <> e") that only hold removal notes for tests like ore_text_equality_operator_finds_match and related JSONB variants; remove these empty banners or consolidate their removal notes into a single explanatory paragraph near the file header/docstring (which currently mentions "equality") so the file’s section structure matches the actual tests; update or delete the three banner comments and move any remaining rationale into one top-level comment adjacent to the existing docstring to keep the file organized.tests/sqlx/tests/equality_tests.rs (1)
85-91: ⚡ Quick winAdd one regression test for the new “missing
hmraises” contract.The comment explains the behavior change, but the file still doesn’t lock it in. Please add a direct assertion that bare
=fails when either side lackshm, so a future fallback-to-scan regression can’t slip back in unnoticed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sqlx/tests/equality_tests.rs` around lines 85 - 91, Add a regression test in tests/sqlx/tests/equality_tests.rs (e.g., fn bare_eq_requires_hm_regression_test) that issues the bare `=` equality query cases and asserts they fail when either operand lacks an `hm` index term: 1) a query where left side has no `hm` and right side does, and 2) a query where right side has no `hm` and left side does; for each case call the same execution path used by the existing equality tests (so it exercises the inlinable operator code path) and assert the result is an Err (or matches the expected error kind/message) rather than succeeding, preventing fallback-to-scan regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/operators/compare.sql`:
- Around line 46-49: Update the Doxygen note about Blake3 in the comment so it
reflects the new call path: replace the statement that "ste_vec_contains calls
eql_v2.compare_blake3 directly" with wording that ste_vec_contains now invokes
eql_v2.eq(...) which may in turn call eql_v2.compare_blake3 as a fallback, and
clarify that Blake3 remains scoped to ste_vec elements and root-level equality
is still HMAC; update references to the functions ste_vec_contains, eql_v2.eq,
and eql_v2.compare_blake3 accordingly.
In `@src/ste_vec/functions.sql`:
- Around line 492-501: The current OR clause uses eql_v2.eq(_a, b) directly,
which lets selector-matched STEs that only carry Blake3 drop to literal
comparison; restore explicit Blake3 matching before the eq fallback by changing
the clause so that when eql_v2.selector(_a) = eql_v2.selector(b) you first check
if both sides carry a b3 (e.g., eql_v2.has_b3(_a) AND eql_v2.has_b3(b)) and, if
so, use eql_v2.compare_blake3(_a, b) = 0 as the match; otherwise fall through to
eql_v2.eq(_a, b). Update the expression around result := result OR (...) to
perform this conditional: selector-equals → (both-have-b3 AND
compare_blake3(...) = 0) OR eql_v2.eq(...).
---
Outside diff comments:
In `@tests/sqlx/tests/index_compare_tests.rs`:
- Around line 19-32: The macro doc for assert_compare! references a stale
example create_encrypted_json(1, 'b3'); update that example to use a currently
emitted filter value (e.g., replace 'b3' with 'hm') so the rationale for using
format! over parameter binding stays correct; locate the macro named
assert_compare! and change the quoted literal in its comment from 'b3' to 'hm'
(or another valid current example) and run the tests to ensure the comment-only
change compiles cleanly.
In `@tests/sqlx/tests/inequality_tests.rs`:
- Around line 173-189: The test
inequality_operator_encrypted_not_equals_jsonb_blake3 duplicates the hmac
variant and is no longer Blake3-specific because create_encrypted_json no longer
emits a root b3 term; either remove this redundant test or rename it to reflect
its generic intent (e.g., inequality_operator_encrypted_not_equals_jsonb) and
update any references; locate the test function
inequality_operator_encrypted_not_equals_jsonb_blake3 and either delete the
entire function or rename it and adjust its test attributes/fixture names
accordingly so it no longer claims Blake3-specific behavior.
In `@tests/sqlx/tests/ore_equality_tests.rs`:
- Around line 1-5: Update the stale module docstring in ore_equality_tests.rs to
describe that the file contains ORE comparison (ordering) tests for <, <=, >, >=
across the ORE schemes (ORE64, CLLW_U64_8, CLLW_VAR_8) and uses the ore table
from migrations/002_install_ore_data.sql; either rename the file to
ore_comparison_tests.rs or move these tests into an existing comparison module
to match the new behavior and avoid future confusion.
---
Nitpick comments:
In `@tests/sqlx/tests/equality_tests.rs`:
- Around line 85-91: Add a regression test in tests/sqlx/tests/equality_tests.rs
(e.g., fn bare_eq_requires_hm_regression_test) that issues the bare `=` equality
query cases and asserts they fail when either operand lacks an `hm` index term:
1) a query where left side has no `hm` and right side does, and 2) a query where
right side has no `hm` and left side does; for each case call the same execution
path used by the existing equality tests (so it exercises the inlinable operator
code path) and assert the result is an Err (or matches the expected error
kind/message) rather than succeeding, preventing fallback-to-scan regressions.
In `@tests/sqlx/tests/ore_text_operator_tests.rs`:
- Around line 13-20: The test file contains empty section headers ("Equality and
inequality operators", "JSONB variants: e = jsonb, e <> jsonb", "JSONB variants:
jsonb = e, jsonb <> e") that only hold removal notes for tests like
ore_text_equality_operator_finds_match and related JSONB variants; remove these
empty banners or consolidate their removal notes into a single explanatory
paragraph near the file header/docstring (which currently mentions "equality")
so the file’s section structure matches the actual tests; update or delete the
three banner comments and move any remaining rationale into one top-level
comment adjacent to the existing docstring to keep the file organized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de2b952e-c56a-4cb2-bae1-7372cae8c504
📒 Files selected for processing (22)
src/encrypted/hash.sqlsrc/operators/<>.sqlsrc/operators/<@.sqlsrc/operators/=.sqlsrc/operators/@>.sqlsrc/operators/compare.sqlsrc/operators/~~.sqlsrc/ste_vec/functions.sqltasks/pin_search_path.sqltasks/test/splinter.shtests/sqlx/migrations/004_install_test_helpers.sqltests/sqlx/tests/constraint_tests.rstests/sqlx/tests/equality_tests.rstests/sqlx/tests/hash_operator_tests.rstests/sqlx/tests/index_compare_tests.rstests/sqlx/tests/inequality_tests.rstests/sqlx/tests/lint_tests.rstests/sqlx/tests/ope_tests.rstests/sqlx/tests/operator_class_tests.rstests/sqlx/tests/operator_compare_tests.rstests/sqlx/tests/ore_equality_tests.rstests/sqlx/tests/ore_text_operator_tests.rs
Comment on lines
+46
to
+49
| --! Blake3 is intentionally not part of the root-level priority list — it is | ||
| --! used only inside ste_vec array elements, where eql_v2.compare_blake3 is | ||
| --! called directly by eql_v2.ste_vec_contains. Root-level equality is hmac | ||
| --! only; see the EQL payload scheme discipline RFC. |
There was a problem hiding this comment.
Update the Blake3 note to match the new ste_vec_contains call path.
This comment says ste_vec_contains calls eql_v2.compare_blake3 directly, but the changed implementation now routes through eql_v2.eq(...). The Doxygen text is stale and hides the new fallback behavior.
Suggested wording
---! Blake3 is intentionally not part of the root-level priority list — it is
---! used only inside ste_vec array elements, where eql_v2.compare_blake3 is
---! called directly by eql_v2.ste_vec_contains. Root-level equality is hmac
+--! Blake3 is intentionally not part of the root-level priority list — it is
+--! only relevant inside ste_vec array elements. Root-level equality is hmac
+--! only; ste_vec containment applies its own element-matching logic before
+--! falling back through eql_v2.eq(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/operators/compare.sql` around lines 46 - 49, Update the Doxygen note
about Blake3 in the comment so it reflects the new call path: replace the
statement that "ste_vec_contains calls eql_v2.compare_blake3 directly" with
wording that ste_vec_contains now invokes eql_v2.eq(...) which may in turn call
eql_v2.compare_blake3 as a fallback, and clarify that Blake3 remains scoped to
ste_vec elements and root-level equality is still HMAC; update references to the
functions ste_vec_contains, eql_v2.eq, and eql_v2.compare_blake3 accordingly.
Comment on lines
+492
to
+501
| -- Compare ste_vec elements via eql_v2.eq, which routes through | ||
| -- eql_v2.compare → literal fallback. Don't use the bare `=` | ||
| -- operator: post-#193 it requires hmac on both sides, which | ||
| -- ste_vec elements don't carry. Don't use compare_blake3 | ||
| -- directly either — its NULL-safe behaviour returns 0 when | ||
| -- both sides lack b3, which would conflate distinct OPE-only | ||
| -- elements. | ||
| result := result OR ( | ||
| eql_v2.selector(_a) = eql_v2.selector(b) AND eql_v2.eq(_a, b) | ||
| ); |
There was a problem hiding this comment.
Restore Blake3 matching before the eq fallback.
After this PR, eql_v2.compare no longer has a Blake3 branch. That means eql_v2.eq(_a, b) now drops to literal comparison for selector-matched STE elements that only carry b3, so two encryptions of the same plaintext can stop matching in @> / <@.
Suggested fix
- result := result OR (
- eql_v2.selector(_a) = eql_v2.selector(b) AND eql_v2.eq(_a, b)
- );
+ result := result OR (
+ eql_v2.selector(_a) = eql_v2.selector(b) AND
+ CASE
+ WHEN eql_v2.has_blake3(_a) AND eql_v2.has_blake3(b) THEN
+ eql_v2.compare_blake3(_a, b) = 0
+ ELSE
+ eql_v2.eq(_a, b)
+ END
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- Compare ste_vec elements via eql_v2.eq, which routes through | |
| -- eql_v2.compare → literal fallback. Don't use the bare `=` | |
| -- operator: post-#193 it requires hmac on both sides, which | |
| -- ste_vec elements don't carry. Don't use compare_blake3 | |
| -- directly either — its NULL-safe behaviour returns 0 when | |
| -- both sides lack b3, which would conflate distinct OPE-only | |
| -- elements. | |
| result := result OR ( | |
| eql_v2.selector(_a) = eql_v2.selector(b) AND eql_v2.eq(_a, b) | |
| ); | |
| -- Compare ste_vec elements via eql_v2.eq, which routes through | |
| -- eql_v2.compare → literal fallback. Don't use the bare `=` | |
| -- operator: post-#193 it requires hmac on both sides, which | |
| -- ste_vec elements don't carry. Don't use compare_blake3 | |
| -- directly either — its NULL-safe behaviour returns 0 when | |
| -- both sides lack b3, which would conflate distinct OPE-only | |
| -- elements. | |
| result := result OR ( | |
| eql_v2.selector(_a) = eql_v2.selector(b) AND | |
| CASE | |
| WHEN eql_v2.has_blake3(_a) AND eql_v2.has_blake3(b) THEN | |
| eql_v2.compare_blake3(_a, b) = 0 | |
| ELSE | |
| eql_v2.eq(_a, b) | |
| END | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ste_vec/functions.sql` around lines 492 - 501, The current OR clause uses
eql_v2.eq(_a, b) directly, which lets selector-matched STEs that only carry
Blake3 drop to literal comparison; restore explicit Blake3 matching before the
eq fallback by changing the clause so that when eql_v2.selector(_a) =
eql_v2.selector(b) you first check if both sides carry a b3 (e.g.,
eql_v2.has_b3(_a) AND eql_v2.has_b3(b)) and, if so, use
eql_v2.compare_blake3(_a, b) = 0 as the match; otherwise fall through to
eql_v2.eq(_a, b). Update the expression around result := result OR (...) to
perform this conditional: selector-equals → (both-have-b3 AND
compare_blake3(...) = 0) OR eql_v2.eq(...).
Introduce a Keep-a-Changelog-style CHANGELOG.md and a Postgres-style per-version upgrade guide under docs/upgrading/. The 2.3.0 entry covers the changes from PR #193 (operator inlining) and PR #196 (payload scheme discipline): the indexing recipe shift from operator-class to functional indexes, the hmac requirement for equality and hashing, and the formalisation of Blake3 as ste_vec-internal. This is a minor release: no public API change. The eql_v2 schema name, function signatures, operators, and payload format are all unchanged.
Add a "Release & changelog discipline" section to CLAUDE.md so the conventions introduced in 41e2925 (CHANGELOG.md + docs/upgrading/v2.3.md) are picked up by AI-assisted contributions without needing a separate CI gate. Covers: when to add a [Unreleased] entry, when to add a numbered upgrade note (U-NNN), how to think about semver in light of the eql_v2 schema name being public API, and the steps for cutting a release.
This was referenced May 9, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Resolves #193. Implements the v2 payload scheme discipline described in
docs/plans/eql-payload-scheme-discipline-rfc.md(forthcoming).Makes the
=,<>,~~,~~*,@>, and<@operator implementations oneql_v2_encryptedeligible for planner inlining. Once inlined, bare queries likeWHERE col = valfrom PostgREST and ORMs that don't wrap columns themselves engage the documented functional indexes (bench_text_hmac_idx,bench_text_bloom_idx,bench_jsonb_stevec_idx) instead of falling back to seq scan.This fixes cipherstash/stack#420 — encryptedSupabase silent seq-scan — at the EQL layer. No changes are needed in
encryptedSupabaseitself.What changed
Operator inlining (commit 1)
src/operators/=.sql,<>.sql,~~.sql: wrapper functions rewritten fromLANGUAGE plpgsql(withSET search_path) toLANGUAGE sql IMMUTABLE STRICT PARALLEL SAFEwith single-statement bodies of the formextractor(a) op extractor(b).src/operators/@>.sql,<@.sql: existingLANGUAGE SQLwrappers gain explicitIMMUTABLE STRICT PARALLEL SAFE(previously default-VOLATILE which blocks inlining).tests/sqlx/tests/lint_tests.rs: tightens the lint test added in #195 with a Phase 1 regression assertion that the targeted operators report zero violations.v2 payload scheme discipline (commit 2)
The operator inlining surfaced that the v2 payload supports multiple parallel SEM terms for the same query purpose at the root, which is structurally incompatible with index matching. Tightening that contract is what makes the inlining actually engage end-to-end:
tasks/pin_search_path.sql: extend the inline-critical allowlist to cover all six Phase 1 operators in all three argument-type permutations ((enc, enc),(enc, jsonb),(jsonb, enc)). Critical: without this, the post-installSET search_pathpin re-disables inlining on the wrappers commit 1 made inlinable. This was the missing piece.eql_v2.compare: drop the Blake3 branch from the equality fallback. Equality at the root is hmac-only; ordering branches (ORE, OPE) unchanged. Blake3 still lives insideste_vec_containsfor selector-level element comparison.eql_v2.hash_encrypted: hmac-only, with a clearer error whenhmis absent. The previous Blake3-first priority existed to satisfy a hash/equality contract that has no production analogue (root payloads never carryb3— Blake3 only appears insidesv[]ste_vec elements).eql_v2.ste_vec_contains: switch the inner element comparison toeql_v2.eq, which routes through compare's literal fallback for ste_vec elements lacking hm/ORE/OPE. Documents why neither bare=norcompare_blake3is appropriate for this internal call site.Test cleanup (commit 2)
tests/sqlx/migrations/004_install_test_helpers.sql: drop the fictional root-levelb3field fromcreate_encrypted_json. ste_vec inner elements still carry b3 viaget_numeric_ste_vec_*.22 tests removed across 9 files because they exercised payload shapes that have no production analogue:
equality_tests: 6 ×equality_operator_*_blake3+ 2 ×eq_function_*_blake3.inequality_tests: 2 ×*_finds_non_matching_records_blake3.index_compare_tests: 3 ×blake3_compare_{equal,less_than,greater_than}.hash_operator_tests: 3 ×mixed_index_*(the fictional hm+b3 vs b3-only contract) +hash_function_uses_blake3_first,hash_consistency_full_index_matches_blake3_only,hmac_and_blake3_produce_different_hashes,ste_vec_wrapped_hashes_same_as_unwrapped.ope_tests: 4 × OPE-only=/<>(OPE is for ordering; equality requires hmac).ore_equality_tests: 6 × ORE-only=/<>(same reason).ore_text_operator_tests: 6 × ORE-text=/<>variants.operator_compare_tests: 1 ×compare_blake3_index(root-level Blake3 compare no longer exists).Two tests in
operator_class_tests.rsrewritten to use hmac literals instead of ob/ore-only literals.add_encrypted_constraint_prevents_invalid_dataswitched to compare'{}'via the underlying jsonb (since the new=requires hmac, and'{}'has none — intentionally).Mechanism
For each operator, inlining produces:
Functional indexes built on the matching expression engage automatically.
Verification
Where the same query previously produced
Seq Scan. The planner inlined=through to the wrapped form, matched the functional hash index, picked Bitmap Index Scan.lint_phase_1_operators_are_cleanasserts zeroinlinability_*violations on the targeted operators against the installed EQL surface.Behavioural change
Per the RFC and the issue,
=/<>previously dispatched througheql_v2.compare, which fell back to ORE / Blake3 / literal comparison when the column's HMAC variant wasn't present. The new implementations require both sides to carryhm. Calling=on a payload missinghmnow raises (Expected a hmac_256 index (hm) value in json: ...) — surfacing the config error rather than silently degrading to seq scan or returning false.For customers configured correctly (the common case — a
uniqueindex gives youhm), this change is purely a perf improvement. The RFC documents the migration-window guidance: during aadd_search_config('unique', ...)rollout against a column that already has rows,eql_v2.eq()(which still walks compare) remains as the explicit predicate for callers that need to span the un-backfilled window.What's NOT in this PR
<,<=,>,>=): need acmp_semextractor design. Tracked separately.->,->>): tracked in Drizzle jsonbPathQueryFirst / jsonbGet typed as predicates but return eql_v2_encrypted stack#423; needs the predicate vs extractor namespace split.hm→e,bf→m, etc.): future RFC.Downstream effect
@cipherstash/drizzle.like/ilikevolatility flip, subsumed.Summary by CodeRabbit
New Features
Bug Fixes
Performance
Documentation
Tests