[3/6] feat: EQL v3 type checker driven by per-column domain capability#424
[3/6] feat: EQL v3 type checker driven by per-column domain capability#424freshtonic 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 |
479744c to
86a8731
Compare
5e9bd60 to
64b7811
Compare
c251fb8 to
3cad2c5
Compare
|
Verified the v3 domain mapping against the actual Findings, most substantive first. 1. 2. 3. Test-fixture footgun from the new identity field. 4. 5. v2 columns → 6. No CHANGELOG entry — CLAUDE.md asks for one on user-facing changes. Not specific to this PR: none of #422/#423/#424/#426/#427/#428 touch CHANGELOG.md, while the queue collectively changes a public signature, swaps the v2 composite for v3 domains, and bumps cipherstash-client 0.34→0.42. Raising against the queue, not this branch. |
tobyhede
left a comment
There was a problem hiding this comment.
Appears to be a working document.
Should this be committed?
|
@tobyhede I believe it should be - it's valuable context |
86a8731 to
21495ac
Compare
…h a real domain Review of the v3 domain-identity work (#424) noted that canonical() can synthesise a name that is not merely approximate but identical to an unrelated real catalog domain — canonical(Text, {json_like}) yields eql_v3_text_search, whose terms are hm/op/bf (Eq+Ord+TokenMatch), nothing to do with JSON. Strengthen the doc comment so no caller mistakes the synthesised name for the column's real domain; only from_domain_name is authoritative. Stable-Commit-Id: q-1ewbmpx74jnyxfd7dyftv08ka9
3cad2c5 to
1a81cf2
Compare
The queue changes user-visible behaviour (v3 typed jsonb domains replace eql_v2_encrypted, cipherstash-client 0.42.0 / EQL 3.0.2, LIKE/ILIKE now capability-checked, `@@` newly supported) but touched no CHANGELOG entry. Add one [Unreleased] section covering the migration, per CLAUDE.md and the review notes on #424/#428. Stable-Commit-Id: q-0s2fx8fb1q2563t7b45x9crjmf
Review of #424 (finding 5) noted the v2->native fallback should be hard to miss in ops, because a v2 column served as a native passthrough takes no encryption on the write path: a plaintext value written to it is stored in plaintext (the eql_v2_encrypted AS ASSIGNMENT cast does no validation). Spell that out in the warning — the column is served as PLAINTEXT, Proxy does no encrypt/decrypt on it, migrate before writing — instead of the softer "ignoring unsupported column". Stable-Commit-Id: q-4dmrmqgewe5mzj0fzb2qdk5dja
Review of #424 (finding 1) flagged that `eql_v3_json_entry` — the element a `->` traversal returns, whose per-entry terms are `hm` XOR `op` — resolved to JsonLike+Contain, because `ste_vec_domain_type!` leaves `term_json_keys()` at None. A column mistakenly declared with that type would have type-checked as supporting `->`/`@>` but not `=`/`<`, which is inverted. Exclude the SteVec sub-structural domains (`NON_COLUMN_DOMAINS`) from the column catalog so such a column falls through to a native (plaintext) column instead of a wrong capability set. The every_public_column_domain assertion skips the same list. Makes the misclassification impossible rather than latent. Stable-Commit-Id: q-4dg749ehycbdazr4etn9rk1fvm
1a81cf2 to
4b016ae
Compare
The queue changes user-visible behaviour (v3 typed jsonb domains replace eql_v2_encrypted, cipherstash-client 0.42.0 / EQL 3.0.2, LIKE/ILIKE now capability-checked, `@@` newly supported) but touched no CHANGELOG entry. Add one [Unreleased] section covering the migration, per CLAUDE.md and the review notes on #424/#428. Stable-Commit-Id: q-0s2fx8fb1q2563t7b45x9crjmf
|
Thanks for the thorough review — went through all six findings. Summary of what changed (all pushed): 1. 2. 3. Test-fixture footgun from the new identity field — Acknowledged, no change. It's loud-not-silent, as you note; leaving it as-is for whoever writes the transform tests next. 4. 5. v2 columns → 6. No CHANGELOG entry — Done (against the queue, per your framing). Added an |
Captures the EQL v3 domain model, three impact maps over eql-mapper (type system, SQL surface, declarations/macros), and the open design questions, so the design session starts fresh with full context. Records one corrected finding: literals are inference sinks (value.rs:19 unifies them with a fresh tvar, not Native), so a token type on EqlValue buys no literal checking. Two subagents disagreed on this; resolved by reading the code. Stable-Commit-Id: q-2r73hgg9ex95aba238a8fbe4kp
Captures the shared understanding from the grill-with-docs session on extending the EQL Mapper type checker for EQL v3. CONTEXT.md: refresh the glossary to the v3 target vocabulary — EqlValue now carries domain identity; EqlTrait drops Contain and is documented as coarse; add term-extraction function, functional-index rewrite, query operand, token type, and domain identity. The old 'known model gap' note becomes 'design decisions in flight' pointing at the ADRs. ADRs (new packages/eql-mapper/docs/adr/): - 0001 functional-index rewrite (term functions, not native operators) — forced by Supabase's lack of CREATE OPERATOR; term-fn selection is the capability check. - 0002 token type as inert identity, not a checked dimension — it buys no safety (encrypted cols never unify; literals are inference sinks). Assessed against EQL/eql-bindings 3.0.2 (released 2026-07-20): additive query-twin bindings only; none of the design decisions change. Stable-Commit-Id: q-06b9k91e7pnntayq3rs11tqznz
…rands Two latent bugs in the trait-bound machinery, harmless only while bound checking is dead code (every encrypted column currently gets EqlTraits::all()). Fix them before the EQL v3 loader makes bounds reachable. - EqlTraits::difference was implemented as XOR (symmetric difference), so UnsatisfiedBounds would list traits the type *has* alongside the ones it is missing. It is now a true set difference (self AND NOT other). - Type::must_implement passed its operands reversed relative to Unifier::satisfy_bounds (implemented.difference(required) instead of required.difference(implemented)). Harmless only because XOR is commutative; wrong the moment difference is corrected. Now consistent. Adds unit tests pinning the asymmetric set-difference semantics and that must_implement reports exactly the missing bounds. Refs CIP-3595. Stable-Commit-Id: q-0e9kbc03hrqef307fg7qb5sqj1
Groundwork for the EQL v3 type checker (ADR-0002). Introduces the token type and its inert per-column domain identity, threading it through the type carriers without making it a checked dimension of unification. - New `TokenType` (integer/text/timestamp/…) and `DomainIdentity` (token type + v3 domain typname, e.g. eql_v3_text_ord_ore). - `EqlValue` widens to (TableColumn, Option<DomainIdentity>, EqlTraits); `ColumnKind::Eql`, `SchemaTableColumn` and the new `Column::eql_with_identity` carry the identity. `Column::eql` stays as a back-compat constructor that defaults the identity to None. - The identity is `None` everywhere for now: the branch still emits the v2 surface and no loader populates it yet (CIP-3598 does). It rides through unification and the associated-type machinery untouched, exactly because it is never inspected there. - ColumnKind loses `Copy` (DomainIdentity is not Copy); the handful of by-value uses now clone. - Also fixes the never-invoked, latently-broken no-bounds arm of the concrete_ty!/EQL() macro (wrong EqlValue arity, missing .into()). eql-mapper suite 79 passing; workspace checks clean; fmt + clippy clean. Refs CIP-3597. Stable-Commit-Id: q-5dxadzsggt5seabqhg3sdj8ht5
Implements the SchemaManager side of the v3 type checker (ADR-0002): per-column capability becomes *observed* from the Postgres domain name instead of a hardcoded EqlTraits::all(). - New schema/eql_domains.rs inverts the eql-bindings v3 catalog (v3::all()) into `typname -> (TokenType, EqlTraits)`. Because the catalog is generated from the same source as the installed SQL domains, the mapping cannot drift from them. Term -> capability: hm->Eq, op/ob->Ord, bf->TokenMatch, empty->storage-only; the JSON SteVec domains (term_json_keys == None) map to JsonLike (provisional, see note). - load_schema now resolves each column's v3 domain identity and traits and builds the column via Column::eql_with_identity. The legacy eql_v2_encrypted composite-type arm is retained for reading pre-v3 schemas; unrecognised domains load as Native. - select_table_schemas.sql now also selects information_schema domain_name: v3 encrypted columns are jsonb-backed DOMAINs, so udt_name reports the base type (jsonb) and the domain typname is only available via domain_name (cf. CIP-3441). - Adds eql-bindings 3.0.1 as a workspace dependency, used only by the proxy's schema loader; eql-mapper stays wire-format-agnostic. Re-exports DomainIdentity / TokenType from eql-mapper's crate root. Unit-tested offline against the catalog (12 tests asserting the SEM-term table, incl. the text hm+ord exception and query-twin exclusion). End-to-end validation needs a database with EQL v3 installed and is deferred; the encrypt-config cross-check is a follow-up within CIP-3598. Refs CIP-3598. Stable-Commit-Id: q-6p9ykbew5b6sjaf5p7y9qza4ft
…+ Contain
Applies three confirmed design decisions on the EQL v3 type checker.
1. Domain identity is non-optional (strict ADR-0002). EqlValue is now
(TableColumn, DomainIdentity, EqlTraits) and ColumnKind::Eql carries a
mandatory DomainIdentity — no more Option. The schema loader always
supplies the real identity; there is no honest v3 identity for a legacy
eql_v2_encrypted column, so the loader now drops the v2 arm and logs a
warning instead of fabricating one (v2 is already retired on this build).
- New helpers: TokenType::{as_domain_str, from_domain_name},
DomainIdentity::{from_domain_name, canonical}, and a test-only
EqlValue::with_canonical_identity.
- The schema!/concrete_ty!/test-helper macros synthesise a canonical
text-token identity by default; schema! also gains an
EQL("<domain>") form to pin an explicit v3 domain (token + OPE/ORE
variant) for tests that care.
2. JSON domains map to JsonLike + Contain, verified against the installed
v3 SQL (cipherstash-encrypt.sql): -> / ->> and @> / <@ are real
operators on eql_v3_json_search (not raise-stubs). Corrects the earlier
JsonLike-only guess.
3. The encrypt-config cross-check is dropped: the domain name is the sole
authority for a column's capability. (ADR-0002 doc update to follow.)
The Contain trait is therefore retained (scoped to JSON), NOT deleted as
the handoff/glossary/ticket assumed — @>/<@ raise only on scalar encrypted
columns. Docs/tickets updated separately.
eql-mapper 79 passing; proxy eql_domains 12 passing; workspace check,
fmt, clippy all clean.
Refs CIP-3597, CIP-3598.
Stable-Commit-Id: q-31sxsrtyrptajjpj0n383cb690
…h a real domain Review of the v3 domain-identity work (#424) noted that canonical() can synthesise a name that is not merely approximate but identical to an unrelated real catalog domain — canonical(Text, {json_like}) yields eql_v3_text_search, whose terms are hm/op/bf (Eq+Ord+TokenMatch), nothing to do with JSON. Strengthen the doc comment so no caller mistakes the synthesised name for the column's real domain; only from_domain_name is authoritative. Stable-Commit-Id: q-1ewbmpx74jnyxfd7dyftv08ka9
Review of #424 (finding 5) noted the v2->native fallback should be hard to miss in ops, because a v2 column served as a native passthrough takes no encryption on the write path: a plaintext value written to it is stored in plaintext (the eql_v2_encrypted AS ASSIGNMENT cast does no validation). Spell that out in the warning — the column is served as PLAINTEXT, Proxy does no encrypt/decrypt on it, migrate before writing — instead of the softer "ignoring unsupported column". Stable-Commit-Id: q-4dmrmqgewe5mzj0fzb2qdk5dja
Review of #424 (finding 1) flagged that `eql_v3_json_entry` — the element a `->` traversal returns, whose per-entry terms are `hm` XOR `op` — resolved to JsonLike+Contain, because `ste_vec_domain_type!` leaves `term_json_keys()` at None. A column mistakenly declared with that type would have type-checked as supporting `->`/`@>` but not `=`/`<`, which is inverted. Exclude the SteVec sub-structural domains (`NON_COLUMN_DOMAINS`) from the column catalog so such a column falls through to a native (plaintext) column instead of a wrong capability set. The every_public_column_domain assertion skips the same list. Makes the misclassification impossible rather than latent. Stable-Commit-Id: q-4dg749ehycbdazr4etn9rk1fvm
Two design corrections found during implementation:
- Containment is NOT removed in v3. Verified against the installed
cipherstash-encrypt.sql: @>/<@ raise on scalar encrypted columns but are
real, supported operators on encrypted JSON (eql_v3_json_search). The
Contain trait is retained as a JSON-only capability. Fixes the glossary
("removed in v3") and adds a consequence to ADR-0001.
- ADR-0002 amended: the domain name is the sole authority for a column's
capability (no encrypt-config cross-check), and the domain identity is
non-optional (legacy eql_v2_encrypted columns are dropped with a warning
rather than given a fabricated identity).
Refs CIP-3597, CIP-3598, CIP-3599.
Stable-Commit-Id: q-4rd3p778qhjfc5dessqeeg1b2e
21495ac to
9c78564
Compare
4b016ae to
3fd4309
Compare
The queue changes user-visible behaviour (v3 typed jsonb domains replace eql_v2_encrypted, cipherstash-client 0.42.0 / EQL 3.0.2, LIKE/ILIKE now capability-checked, `@@` newly supported) but touched no CHANGELOG entry. Add one [Unreleased] section covering the migration, per CLAUDE.md and the review notes on #424/#428. Stable-Commit-Id: q-0s2fx8fb1q2563t7b45x9crjmf
📚 eql-v3 PR · 3 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).✅🟢 #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👈 this PR✅🟢 #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✅ 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
Type checker for v3 domains — extends EQL Mapper to emit
eql_v3_<token>_<capability>domains instead of eql_v2_encrypted(designed, not yet built; the step that makes the migration actually work).