[6/6] chore(deps): upgrade EQL and eql-bindings to 3.0.3 - #430
[6/6] chore(deps): upgrade EQL and eql-bindings to 3.0.3#430freshtonic wants to merge 10 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
9d67d39 to
280a0a3
Compare
c55f621 to
b4d92ec
Compare
Encrypted JSON equality (`col -> sel = value`) is not a term comparison. In EQL v3 exact equality is *containment of a value selector*: a single keyed MAC over the path and the canonicalised value together (`QueryOp::SteVecValueSelector`). One needle, built from TWO SQL operands. This is the mapper half. It breaks the implicit 1:1 correspondence between input and output operands for the first time: - New `EqlTerm::JsonValueSelector` / `EqlTermVariant::JsonValueSelector` — the value operand of a JSON field equality. Inferred for the non-JSON side of `=`/`<>` whose other side is a genuine field ACCESS (`->`, `->>`, `jsonb_path_query_first`). A bare `col = $1` on a whole encrypted JSON column is document equality and keeps its ordinary typing. - New `JsonValueSelectors` on `TypeCheckedStatement` records the one relationship node types cannot express: which operand supplies the path for which value. The mapper holds no encryption key, so it emits the composition *input*, not the needle; the proxy fuses and encrypts. - New `RewriteJsonValueSelectorEq` rewrites the comparison to `eql_v3.jsonb_contains(col, <value>::eql_v3.query_json)` (`<>` negates), discarding the field access. `RewriteEqlComparisonOps` now skips these — `eql_v3.eq_term` has no unique overload for a JSON query operand. - The value operand casts to `eql_v3.query_json`, alongside the existing `JsonOrd` -> `query_integer_ord` rule (helper renamed to `json_query_operand_cast_target` to cover both). A discarded selector *placeholder* stays declared in Parse but unreferenced in the SQL. PostgreSQL permits that as long as its type is known, which is what lets input and output param numbering stay identical — so Bind, ParameterDescription and the encrypt pipeline all stay positional. Verified against PostgreSQL 17: `PREPARE p (text) AS SELECT $2::jsonb` prepares and executes; the same statement with no declared types fails with "could not determine data type of parameter $1". Mapper unit tests 97 -> 102, no regressions. Proxy-side composition (the literal and Bind paths) follows in subsequent commits. Stable-Commit-Id: q-7qfnv43maha2chq6shq55sc3m0
The proxy half of encrypted JSON field equality. The mapper types the
value operand `JsonValueSelector` and records where its path comes from;
this fuses the two SQL operands into one `{"path", "value"}` plaintext and
encrypts it with `QueryOp::SteVecValueSelector`, producing the one-entry
containment needle `eql_v3.jsonb_contains` matches.
Both protocols:
- Simple: `literals_to_plaintext` composes the needle for a value-selector
literal from its own JSON scalar and the selector literal's text. The
discarded selector literal is still encrypted but never placed, since the
rewrite removed its node.
- Extended: `Bind::to_plaintext` gains a second pass. A fused param is
skipped by the positional pass — its own bytes are only half a needle and
would not decode as a standalone operand for the column — then composed
from the path param (or literal) and its own value.
- `Parse::declare_unreferenced_param_types` declares TEXT for the selector
placeholder the rewrite dropped. Clients that let the server infer types
send no types at all, and PostgreSQL will not prepare a statement with an
unreferenced param of unknown type. Still-referenced slots are left at
OID 0 ("infer"), so this never overrides a type the query determines.
Param counts are unchanged end to end, so Bind, ParameterDescription and
the encrypt pipeline all stay positional.
Also folds the four copies of the `$.`-rooting of a JSON selector into one
`json_selector_path` helper — the fused path needs the same normalisation
the accessor and path-query operands already did.
Fixes `select_jsonb_where_{string,numeric}_eq` (both protocols, both the
`->` and `jsonb_path_query_first` spellings). jsonb suite 70 -> 72
passing. Verified zero regressions by running the full integration suite
against the base binary and diffing failure sets: the only differences are
these two tests plus known-flaky ORE ordering tests that fail identically
on base.
The 7 `jsonb_term_filter` tests remain red for an unrelated reason:
`encrypted_jsonb_filtered` is declared `eql_v3_json_search`, the same
domain as the unfiltered column, and `from_domain.rs` hardcodes
`term_filters: Vec::new()`. EQL v3 has no domain or config that encodes a
term filter, so the proxy cannot infer one. Equality itself works on that
column — it matches case-sensitively. Case-insensitive JSON needs an
EQL-side way to declare filters; that is a separate feature.
Stable-Commit-Id: q-5afqdbdnjhd299x8777mwdm4fj
Stable-Commit-Id: q-6658b8n0t0qbfkcksk6720x0b5
Replaces the dangling-param trick with the real shape of the problem: an output param may be derived from more than one input param, so there is no 1:1 correspondence to rely on. Previously the JSON-equality rewrite left the fused path operand in the SQL as an unreferenced `$n`, purely so that param numbering stayed positional. That kept the plumbing simple at the cost of lying about the statement: a param that means nothing, and a neighbouring param whose meaning has quietly become "composite". Both are invisible at the call sites that bind them. The correspondence is now explicit: - `TypeCheckedStatement::transform` returns a `TransformedStatement` — the rewritten SQL plus a `ParamPlan`. Each `OutputParam` names the input param(s) its value is built from (`Input`, or `JsonValueSelector` carrying both operands). - A `RenumberParams` pass runs after the rewrite rules and assigns `$1..$m` in SQL order, so rules may freely drop or duplicate placeholders without maintaining numbering themselves. Running it as a separate pass keeps `FailOnPlaceholderChange` governing the rules. - `ParamPlan::check_covers` enforces the invariant that replaces 1:1 — **coverage**: every input param must be consumed by some output. An input that feeds nothing has been silently dropped, which is a bug in a rewrite rule. The proxy binds against the plan rather than by position. `Statement` carries both shapes: `param_columns` (what the client binds, used to decode and to answer Describe) and `output_params` (what PostgreSQL receives). Bind builds each output from the inputs its source names; ParameterDescription is rebuilt from the input params, since the server describes the rewritten statement and would otherwise tell the client too few params to bind. Parse carries each client-declared type across to the output param that consumes it. Every output param is now referenced by the rewritten SQL, so `declare_unreferenced_param_types` and its unreferenced-placeholder workaround are gone. When the plan is positional — every statement that is not a JSON equality — Bind still patches values in place, leaving the client's wire framing byte-for-byte as sent. Mapper units 102 -> 104 (renumbering across a fusion, and identity plans); proxy units 116 -> 117. Integration unchanged: the 2 JSON equality tests pass, and the four ORE-ordering tests that differ from the previous run fail identically on the base binary when run in isolation. Stable-Commit-Id: q-67avvp278knhdy7xe192mq667s
Removes `CastLiteralsAsEncrypted` and `CastParamsAsEncrypted`. Both were context-blind: they fired on any encrypted literal or placeholder and then guessed which domain to cast it to by walking up the ancestor chain (`is_query_operand`) looking for a comparison or a LIKE. The rule that actually knew the context — the one rewriting the predicate — was a different rule entirely, running later. Now the rule that owns a construct casts the operands of that construct, where the context is a fact rather than an inference: - `RewriteEqlComparisonOps`, `RewriteEqlMatchOps` — query operands, cast to the term-only `eql_v3.query_*` twin. - `RewriteJsonValueSelectorEq` — the fused needle, `eql_v3.query_json`. - `RewriteContainmentOps` — a `@>`/`<@` needle is a whole document, so it casts to the column domain; a `->`/`->>` selector takes no cast. - `CastFullPayloadOperands` (new) — the contexts with no rewrite of their own: INSERT values, UPDATE assignments, and directly-written `eql_v3.jsonb_contains(col, $1)` calls, which clients on platforms without operator support write themselves and which need the column-domain cast for the GIN index over `eql_v3.jsonb_array(col)`. It fires on the enclosing `Values`/`Assignment`/`Function` node, not on the literal, so the context is again structural. Substitution is separated from casting: `SubstituteEncryptedLiterals` puts each ciphertext in place wherever it appears and casts nothing, since where a literal sits decides its domain but not whether it needs replacing. It keeps the postcondition that every encrypted literal was consumed. The two domain choices are now named for the distinction they encode — `query_operand_domain` (just the terms a predicate needs) and `full_payload_domain` (ciphertext plus every indexed term) — and `is_query_operand`, the ancestor walk, is gone. Every rule matches against the ORIGINAL node via `node_path`, so a construct already rewritten by an earlier rule cannot be cast twice. Mapper 104 unit tests and proxy 117 unchanged. Integration: no regressions — the only tests that differ from the previous run are in the flaky ORE/OPE ordering family (net +1 passing), and the one that regressed both appears in the base binary's failure list and flips between ok and failed across identical isolated runs. Stable-Commit-Id: q-3y30vhx105f1ms7h36tanr8k7m
…term
`ORDER BY col` on an encrypted column was passed through untouched. The
column is a domain over `jsonb`, so PostgreSQL compared whole payloads by
jsonb rules — and jsonb compares objects field by field starting at `c`,
the ciphertext, which is randomised per encryption. Results came back in
an order that was not merely wrong but different on every insert.
Rewrites to the ordering term, which is what makes the comparison
meaningful:
ORDER BY col -> ORDER BY eql_v3.ord_term(col)
ORDER BY t.col DESC -> ORDER BY eql_v3.ord_term(t.col) DESC
ORDER BY col NULLS FIRST-> ORDER BY eql_v3.ord_term(col) NULLS FIRST
(`ord_term_ore` for block-ORE domains.) Sort options are untouched, so
ASC/DESC and NULLS FIRST/LAST keep working — the term is a plain orderable
value and NULL stays NULL. `ord_term` yields `ope_cllw`, a `bytea` domain
ordered bytewise; `ord_term_ore` yields `ore_block_256`, ordered by its own
btree operator class. Verified directly against PostgreSQL: over four
values re-encrypted five times, the bare `ORDER BY` gave a different wrong
order every run while the term ordering was correct every run.
Ordering by a column whose domain carries no ordering term is now a
capability error rather than an arbitrary sort, matching how
`RewriteEqlComparisonOps` treats an unsupported operator.
This is the cause of the "flaky" ORE/OPE ordering tests. They were not
flaky: the ordering was random, so a test comparing two values passed
about half the time while one comparing five values essentially never did.
Integration: 236 -> 298 passing, 168 -> 106 failing, nothing newly
failing. Fixes all of map_ope_index_order (6), map_ore_index_order (18)
and select::order_by (42), and those families are now deterministic across
repeated runs. Mapper units 104 -> 107.
Stable-Commit-Id: q-7znx6wzcc63yyk8hnganvrbdce
Every scalar predicate on an encrypted column failed:
ERROR: value for domain eql_v3.query_integer_ord violates check
constraint "query_integer_ord_check"
A query operand carries the column's search terms but never a decryptable
ciphertext — the `eql_v3.query_*` domains enforce that with `NOT (VALUE ?
'c')`. The proxy encrypted every scalar operand with `EqlOperation::Store`
and sent the storage payload, `c` and all, into a query position.
It cannot simply encrypt differently: a single `EqlOperation::Query`
yields only ONE term, while an operand may need several (`{v,i,hm,ob}` for
`query_text_ord`). The client's answer is to encrypt in Store mode and
project — `EqlCiphertextV3::into_query_operand`, whose own test is named
`into_query_operand_scalar_keeps_all_terms_and_drops_c`. The proxy never
called it.
Projecting needs to know which operands are query operands, and nothing
carried that: the operand of `WHERE col = $1` and the value of
`INSERT ... VALUES ($1)` are both `EqlTerm::Full`. So the mapper now
records it, in the new `QueryOperands`, alongside the existing
`JsonValueSelectors`. Membership is decided syntactically by the predicate
an operand belongs to — comparisons, `LIKE`/`ILIKE`, `@@` — which is the
same set of contexts whose rewrite rules cast to a `eql_v3.query_*` twin.
Containment (`@>`/`<@`) is deliberately excluded: its needle is a whole
document and keeps its full payload, as do INSERT values and UPDATE
assignments.
The flag rides through `ParamPlan` to the proxy's `OutputParam`, so Bind
projects the params it sends, and the literal path projects at Parse.
Payloads the encryptor already produced query-shaped (the JSON selectors
and SteVec terms, which do go through `EqlOperation::Query`) are left
alone.
Fixes map_ope_index_where (6), map_ore_index_where (6), map_match_index
(1) and 7 of 8 map_unique_index — scalar encrypted search worked in
neither protocol before this. Integration 298 -> 317 passing, 106 -> 87
failing, nothing newly failing.
The remaining map_unique_index failure (`..._all_with_wildcard`, a
`SELECT *` with six encrypted params) fails with a protocol-level
UnexpectedMessage and is a separate, pre-existing bug.
Stable-Commit-Id: q-4jyyztp3wxv6ve8ndnwpp59r2y
…term
The third instance of the same trap. An encrypted column is a domain over
`jsonb`, so a bare `GROUP BY col` grouped on the whole payload — including
`c`, the ciphertext, which is randomised per encryption. Rows holding the
same plaintext landed in different groups, degrading `GROUP BY` into
`GROUP BY <every row>`: `select::group_by` saw 20 groups for 10 distinct
values.
Grouping is equality, so the key is the same term `=` uses —
`eql_v3.eq_term(col)`, or `ord_term` for a domain that stores no `hm`
(`eql_v3_integer_ord` groups through its ordering term, exactly as it
compares).
Once the key is `eq_term(col)`, PostgreSQL no longer sees the bare column
as functionally dependent on it and rejects `SELECT col`. So a projection
of a grouped column is lifted through an aggregate, as James specified:
SELECT col, COUNT(*) FROM t GROUP BY col
->
SELECT any_value(col) AS col, COUNT(*) FROM t GROUP BY eql_v3.eq_term(col)
Every row in a group holds the same plaintext, so any of them decrypts to
the right answer. `FIRST` does not exist in PostgreSQL and `min`/`max` are
not defined over `jsonb`; `any_value` is polymorphic and works on the
domain directly. It requires PostgreSQL 16+, but only for the projection
case — grouping without selecting the column works on any version.
The projection is matched on the resolved column rather than on syntax, so
`SELECT t.col ... GROUP BY col` — which PostgreSQL accepts today — keeps
working. The original projection name is preserved, so clients selecting
by name are unaffected: this is what keeps
`select::regression::select_with_order_by_in_subquery` (a
`SELECT encrypted_text ... GROUP BY encrypted_text`) passing, which a
group-key-only rewrite would have broken.
Grouping by a column whose domain carries no equality term is now a
capability error rather than a grouping on ciphertext.
Integration 317 -> 323 passing, 87 -> 81 failing, nothing newly failing.
Mapper units 107 -> 109.
Stable-Commit-Id: q-4080dw66nm5aqm7g0q717mqvmw
Replaces the `any_value` stand-in with the aggregate EQL provides for
exactly this case (CIP-3657, EQL PR 423):
SELECT col, COUNT(*) FROM t GROUP BY col
->
SELECT eql_v3.grouped_value(col) AS col, COUNT(*)
FROM t GROUP BY eql_v3.eq_term(col)
`grouped_value` returns one representative value per group without
decrypting or comparing anything, which is all that is needed: every row
in a group is an encryption of the same plaintext. It re-creates the
aggregate the eql_v2 surface provided and that 3.0.0 dropped, and it
accepts any encrypted-domain value since all of them are jsonb-backed.
Being EQL's own answer to this problem it is also the form EQL's docs
describe, so a hand-written query and a proxy-rewritten one now agree.
`any_value` worked but was the wrong dependency to take: it is a
PostgreSQL 16+ builtin, so it silently raised the proxy's minimum server
version for a case EQL already covers.
**Requires an EQL release later than the 3.0.2 pinned in `mise.toml`** —
`eql_v3.grouped_value` does not exist in 3.0.2, so `select::group_by` and
`select::regression::select_with_order_by_in_subquery` will fail on a
clean install until `CS_EQL_VERSION` is bumped. Verified here by
installing the aggregate from PR 423 into the dev database by hand: all 6
group_by tests and the regression test pass, and the full integration
suite is unchanged at 323 passing / 81 failing with nothing newly failing.
Stable-Commit-Id: q-4h7rjf4sncgcvk7v1cpy3ndxjd
`CS_EQL_VERSION` eql-3.0.2 -> eql-3.0.3, and the `eql-bindings` crate =3.0.1 -> =3.0.3. EQL 3.0.3 carries `eql_v3.grouped_value` (CIP-3657, EQL PR 423), which the GROUP BY rewrite emits. Until now that commit was red on a clean install — the aggregate did not exist in 3.0.2 and had to be applied to the dev database by hand to test. It is now installed by the release, so `select::group_by` and `select::regression` pass with no local patching. `eql-bindings` was pinned two releases behind the SQL at 3.0.1; 3.0.2 and 3.0.3 are both the same ts-rs serde-attribute warning fix, with the emitted TS/JSON byte-identical, so the jump carries no behaviour change. The proxy uses the crate only in its schema loader, to map v3 domains to capabilities. Integration suite unchanged at 323 passing / 81 failing, with nothing newly failing or passing. Mapper 109 and proxy 117 unit tests green. Stable-Commit-Id: q-67fwe28n0ab44t0zefphhnqd4p
280a0a3 to
68f56d3
Compare
b4d92ec to
6c6ead8
Compare
|
Blacksmith runners detected OOM events on the following jobs:
|
📚 eql-v3 PR · 6 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) andgit queue submit(retargets their PRs).✅🟢 #423
queue/eql-v3/upgrade-deps→main♻️🟢 #424
queue/eql-v3/typecheck→queue/eql-v3/upgrade-deps✅🟢 #428
queue/eql-v3/transform→queue/eql-v3/typecheck✅🟢 #426
queue/eql-v3/showcase→queue/eql-v3/transform⏳🟢 #427
queue/eql-v3/integration→queue/eql-v3/showcase⏳🟢 #430
feat/eql-v3-jsonb-composite-selectors→queue/eql-v3/integration👈 this PR✅ 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.
About this branch
Encrypted JSON field equality (
col -> field = value), plus the correctness fixes it surfaced.Equality on a JSON field is value-selector containment: one keyed MAC over the path and the value together, so two SQL operands fuse into one encrypted needle. That breaks the implicit 1:1 correspondence between input and output params, so the mapper now returns an explicit
ParamPlan— each output param naming the input params it derives from — and the proxy binds against that rather than by position. Casting moved out of the two context-blind cast rules and into the rewrite rule that owns each construct.Working on it exposed three cases where SQL the mapper did not rewrite was silently operating on the ciphertext rather than on search terms, returning wrong results without erroring:
ORDER BY(ordering was random per insert — the cause of the "flaky" ORE tests),GROUP BY(equal plaintexts landed in different groups), and every scalarWHERE(the storage payload was sent where a query operand was required). Each is fixed here.Integration suite 236 -> 323 passing. Requires EQL 3.0.3 for
eql_v3.grouped_value, bumped in the last commit.