[4/6] feat(eql-mapper): rewrite SQL to the EQL v3 functional-index form

[freshtonic]

📚 eql-v3 PR  ·  4 of 6

Part of a queue. The PRs merge in FIFO order — the numbered order below, #1 first. Merging one supersedes the PRs after it until the author runs git queue sync (rebases the rest onto the merged base) and git queue submit (retargets their PRs).

✅🟢 #422 queue/eql-v3/setup-skills → main
✅🟢 #423 queue/eql-v3/upgrade-deps → queue/eql-v3/setup-skills
♻️🟢 #424 queue/eql-v3/typecheck → queue/eql-v3/upgrade-deps
✅🟢 #428 queue/eql-v3/transform → queue/eql-v3/typecheck  👈 this PR
✅🟢 #426 queue/eql-v3/showcase → queue/eql-v3/transform
⏳🟢 #427 queue/eql-v3/integration → queue/eql-v3/showcase

_{✅ approved · ♻️ changes requested · ⏳ review pending  |  🟣 merged · 🟢 open · ⚫ closed  —  status as of the last git queue submit.
🥞 Managed by git-queue — do not edit this list by hand.}

About this queue

Migrates CipherStash Proxy from EQL v2 to EQL v3 (cipherstash-client 0.34.1-alpha.4 → 0.42.0, EQL 2.3.0-pre.3 → 3.0.2), replacing the opaque eql_v2_encrypted composite type with 53 typed jsonb domains that encode both scalar type and searchable capability in the column type itself.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc307676-9f0d-4630-906e-fe223fc52d94

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch queue/eql-v3/transform

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[tobyhede]

Review — v3 rewrite pipeline

Careful, well-documented implementation. ADR-0003 is genuinely load-bearing, the domain-capability logic is exhaustively unit-tested, and the mem::replace-against-Null NodeKey-preservation trick is applied consistently. Verified correct: post-order rule composition (operands are cast before the enclosing predicate wraps them), is_query_operand correctly excluding containment (@>/<@ operands cast to the public column domain, comparison/match operands to the eql_v3.query_* twin), and the eq_term→ord_term fallback / ord_term vs ord_term_ore selection.

Nice catch on the LIKE capability bypass — the old arm just did unify_node_with_type(expr_val, native), so LIKE on an Eq-only encrypted column was silently accepted. Also confirmed routing native LHS through the T: TokenMatch-bounded rule is safe (satisfy_bounds rule 1: Native satisfies all bounds), so WHERE native_col LIKE 'x' is not regressed.

🔴 Blocking-ish

1. The JsonAccessor selector assumption is unresolved and untestable as written. cast_literals_as_encrypted.rs carries its own NOTE: "the encrypted replacement is the selector string itself. If it is instead a jsonb payload, this needs the selector extracted rather than emitted verbatim." The ->/->> test can't settle it — the replacement is the stub '<encrypted-selector(medications)>', which is indistinguishable from a jsonb payload. If the encrypt pipeline returns a payload, eql_v3."->"(notes, <payload>) is wrong SQL. Please confirm against the real encrypt path before merge.

2. No execution anywhere. CONTEXT.md says it: "End-to-end validation against a live database with EQL v3 installed is still pending." Every test is assert_eq!(statement.to_string(), "…"). Each emitted name (eql_v3.ord_term, match_term, query_text_ord, the public.eql_v3_* domains, match_term(a) @> match_term(b)) is verified only against a reading of cipherstash-encrypt.sql. One wrong name = runtime failure this suite cannot see. Compounded by CI: gh pr checks shows only CodeRabbit ("reviews are disabled for this base branch") — no build or test job on the queue base.

🟡 Test gaps (all verified absent)

• NOT LIKE / NOT ILIKE — zero occurrences. The Expr::UnaryOp{Not, Nested(..)} negation branch in rewrite_eql_match_ops.rs is entirely unexercised.
• ILIKE — zero occurrences (only LIKE and @@ have rewrite-output tests).
• UPDATE SET stored-value cast — update / update_with_returning_clause exist but only type-check and assert typed.projection; neither calls .transform(). So of ADR-0003's two stored-value contexts, only INSERT has cast-target coverage. This is the load-bearing new concept in the ADR — worth pinning.
• native LIKE — no regression test locking in that WHERE native_col LIKE 'x' still type-checks (it does; I verified).
• Transform-time capability errors — the "does not support operator" / "does not support fuzzy match" branches in both new rules are untested.

🟡 Question — double precision domain naming

suffix() computes prefix_len = "eql_v3_".len() + token.as_domain_str().len(), which relies on the token being a single underscore-free word (as from_domain_name's split('_').next() assumes). That holds for every token except possibly double: if the real domain is eql_v3_double_precision_ord, suffix() returns "precision_ord", every stores_* predicate returns false, and the column silently reports as non-orderable — no error, just a missing rewrite. If the domain is eql_v3_double_ord, no issue. Worth a test either way; the invariant is currently a comment, not an assertion.

🟠 Process

Please retitle. docs(eql-mapper): cleanup … stale strings and notes describes ~125 of 1004 added lines. The rest is ~618 lines of functional code — two new transformation rules (rewrite_eql_comparison_ops.rs +123, rewrite_eql_match_ops.rs +137), helpers.rs +93 (is_query_operand, v3_cast_target, eql_v3_term_call), domain-capability methods in types.rs, a new @@ operator across parser macro + operator table + inference, and the LIKE capability fix — plus ~261 lines of tests. feat/refactor would be accurate; as-is this undersells the change to reviewers and to any changelog tooling.

Relatedly, no CHANGELOG entry despite user-visible behavior changes (LIKE is now capability-checked and will reject columns that previously slipped through; @@ is newly supported). Confirm whether the queue handles that at merge.

[freshtonic]

Addressed the review (all pushed):

• Test gaps: added rewrite-output coverage for the previously-untested cases — ILIKE, NOT LIKE / NOT ILIKE (the negated NOT (…) match arm), the UPDATE SET stored-value cast (SET salary = 'ENCRYPTED'::JSONB::public.eql_v3_text — only INSERT had cast-target coverage), a native-LIKE regression lock (Native satisfies all bounds), and the double-token suffix invariant (eql_v3_double_ord, which the reviewer flagged as comment-only). 96 mapper tests green. → test(eql-mapper): cover NOT LIKE/ILIKE, UPDATE SET cast, native LIKE, double domain
• CHANGELOG: added the [Unreleased] entry the review asked for (here and on [3/6] feat: EQL v3 type checker driven by per-column domain capability #424), covering the migration — v3 typed jsonb domains, cipherstash-client 0.42 / EQL 3.0.2, LIKE/ILIKE capability-checking, and @@. → docs(changelog): record the EQL v3 migration
• Retitle to feat/refactor: not done — that's a commit-message rewrite, so leaving it for the merge rather than rebasing the stack.
• Transform-time capability-error branches: these are unreachable through the public type_check → transform path (the type checker rejects the operator/fuzzy-match before the rewrite backstop can fire), so they can't be exercised by a normal unit test — left uncovered rather than adding white-box hacks.
• JsonAccessor selector confirmation & end-to-end execution: still need a live EQL v3 DB, so they remain open — not settleable from the code alone.

[tobyhede]

Review notes. Verified against the shipped EQL 3.0.2 cipherstash-encrypt.sql and the generated eql-bindings 3.0.1 catalog, plus probe statements run through type_check + transform on this branch.

Blocking

1. Every rewritten predicate raises a domain CHECK violation.

CREATE DOMAIN eql_v3.query_text_eq AS jsonb
  CHECK (... AND VALUE ? 'hm' AND NOT (VALUE ? 'c') AND VALUE->>'v' = '3');

Query operands are EqlTerm::Full (still asserted in basic_with_value), and zerokms.rs:250 maps Full → EqlOperation::Store, so the payload carries c. This PR casts it to eql_v3.query_*. Domain CHECKs fire on cast ⇒ =, <>, <, <=, >, >=, LIKE, ILIKE, @@ on an encrypted column all fail at the database. #427 does not change the Full → Store mapping.

2. ORDER BY / GROUP BY / DISTINCT are not rewritten — silent wrong results, and a v2 regression.

EQL v2 shipped CREATE OPERATOR CLASS eql_v2.encrypted_operator_class DEFAULT FOR TYPE eql_v2_encrypted USING btree. v3 creates a btree opclass only for the internal ore_block_256 type, none for the public domains, so ORDER BY col falls back to jsonb's opclass and sorts by ciphertext structure without erroring. Needs ORDER BY eql_v3.ord_term(col). Confirmed by probe: SELECT id FROM employees ORDER BY salary DESC passes through unchanged. Note lib.rs:970 already exercises order by salary desc but only type-checks.

3. ->/->> selector: literal and param paths disagree.

literal: eql_v3."->"(notes, 'ENC')
param:   eql_v3."->"(notes, $1::JSONB::public.eql_v3_text_search)

Signature is eql_v3."->"(e public.eql_v3_json_search, selector text), so the literal path is right and CastParamsAsEncrypted needs the same EqlTerm::JsonAccessor arm CastLiteralsAsEncrypted has. This also resolves the in-code "NOTE (assumption to confirm against the encrypt pipeline)" — the assumption holds.

ADR-0003's premise

ADR-0003 justifies the functional-index form on managed Postgres forbidding operator DDL. CREATE OPERATOR is not restricted — v3 ships 3029 of them, including on the public column domains with (domain,domain), (domain,jsonb) and (jsonb,domain) overloads. What requires superuser is CREATE OPERATOR CLASS, which v3 handles via the ORE-fallback poison constraints.

• = <> < <= > >= → native operators exist; RewriteEqlComparisonOps is redundant.
• @@ → 207 native operators; that arm is redundant.
• ~~ / ~~* → zero operators; LIKE/ILIKE genuinely needs the rewrite, and the capability-gating fix is correct.

Smallest correct change is probably: drop the comparison rewrite and the @@ arm, keep LIKE/ILIKE → match_term(col) @> match_term(pat), cast all operands to the column domain. That removes issue 1 entirely. Keeping the twins requires a scalar EqlOperation::Query on the encrypt side first, which means the mapper must stop reporting operands as Full.

Verified fine — no action

IN, BETWEEN, IS DISTINCT FROM are untouched by is_query_operand, so they keep the column-domain cast (Store payload satisfies the CHECK) and resolve to the native domain operators. They work.

Minor

• I checked all 50 catalog column domains against DomainIdentity::stores_hm/op/ob/bf: 50/50 agree, including the text_ord* hm exception. The heuristic is a second derivation of what eql_domains.rs gets from the catalog, so it's a drift hazard rather than a live bug — a test asserting agreement across v3::all() would pin it. Worth flagging that [6/6] feat: infer v3 encrypt config and convert the integration suite #427 deletes double_token_does_not_swallow_the_capability_suffix and waters down the canonical() warning doc.
• suffix() returns "" on any prefix mismatch, silently reporting zero capabilities. Prefer an error.
• CastParamsAsEncrypted::apply returns Ok(false) where CastLiteralsAsEncrypted returns Err for the same condition. Unreachable today; make them consistent.
• eql_v3_term_call and RewriteContainmentOps::make_function_call are near-duplicate Expr::Function builders.
• count is no longer rewritten (v2 emitted eql_v2.count). Looks intentional and correct, but it's an unmentioned behaviour change.
• NOT LIKE → NOT (match_term(a) @> match_term(b)) inherits bloom-filter false positives, so it can drop genuinely non-matching rows. Behaviour, not a defect — worth a CHANGELOG line next to the LIKE gating fix.

gh pr checks 428 shows no build or test job on this PR, which is how 1 and 2 got this far.