Skip to content

feat: native-spelling encrypted-domain aliases (int4/int8/int2/float4/float8/decimal)#381

Open
tobyhede wants to merge 16 commits into
mainfrom
eql-v3-encrypted-domain-aliases
Open

feat: native-spelling encrypted-domain aliases (int4/int8/int2/float4/float8/decimal)#381
tobyhede wants to merge 16 commits into
mainfrom
eql-v3-encrypted-domain-aliases

Conversation

@tobyhede

@tobyhede tobyhede commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Adds Postgres-native type-spelling aliases to the EQL v3 encrypted-domain surface, generated from the catalog. Each alias is a full standalone encrypted type — not a view, cast, or wrapper — that shares the canonical payload envelope exactly and interoperates with its canonical twin in both directions via generated cross-name operators.

Canonical Alias Canonical Alias
smallint int2 real float4
integer int4 double float8
bigint int8 numeric decimal

Usage

An alias carries the same per-capability variants as its canonical family (public.<name>, _eq, _ord, _ord_ore, _ord_ope). Type columns with the schema-qualified name:

CREATE TABLE payments (
  id      bigint,
  amount  public.int8_ord,   -- ordered encrypted column; identical to public.bigint_ord
  ref     public.int4_eq     -- equality encrypted column;  identical to public.integer_eq
);

-- Index through the extractor, exactly as for the canonical type:
CREATE INDEX payments_amount_ord ON payments USING btree (eql_v3.ord_term(amount));
CREATE INDEX payments_ref_eq     ON payments USING hash  (eql_v3.eq_term(ref));

Both-directions interop

An alias value and its canonical twin compare/join correctly, in either operand order. The comparison binds the generated cross-name operator, routing through the encrypted index term (HMAC for equality, ORE/OPE for ordering) — never native jsonb:

-- int4-typed value vs integer-typed value: HMAC equality, not raw-ciphertext jsonb =
SELECT * FROM a JOIN b ON a.ref = b.legacy_ref;   -- a.ref public.int4_eq, b.legacy_ref public.integer_eq

-- function form (platforms without operator support, e.g. Supabase/PostgREST):
SELECT eql_v3.eq(a.ref, b.legacy_ref);

-- ordered comparison across the alias boundary:
SELECT * FROM a WHERE a.amount < b.threshold;     -- public.int8_ord  vs  public.bigint_ord

Two independent encryptions of the same value therefore compare equal across the alias boundary — native jsonb = (comparing the raw ciphertext c) would wrongly report them distinct. That silent degradation is exactly what the cross-name operators shadow.

Value conversion is a plain cast

SELECT $1::jsonb::public.int4_eq;   -- the same envelope, re-typed as the alias (no re-encryption)

No CREATE CAST (impossible on domains, and unnecessary — both resolve to jsonb).

⚠️ Always schema-qualify

Every one of these names — aliases and canonical spellings — is also a built-in PostgreSQL type, resolved from pg_catalog, which shadows public. A bare unqualified name binds the plaintext built-in, not the encrypted domain:

CREATE TABLE t (x int4);          -- ❌ plaintext built-in integer — NOT encrypted
CREATE TABLE t (x public.int4);   -- ✅ the encrypted-domain alias

This is not alias-specific — public.integer behaves identically. Documented in docs/reference/aliases.md.

How it's generated

One aliases field on the catalog's DomainFamily drives everything. group_names() (canonical-first) is the single fan-out site; the generator emits a full per-name surface under src/v3/scalars/<name>/, plus a cross-name operator file for every unordered pair of group names — shadowing exactly the 9 operators with a Domain/Domain signature (= <> < <= > >= @> <@ ||), classified from the operator table, not a hardcoded list. Supported ops become public wrappers (eql_v3.*); unsupported ones become internal plpgsql blockers that raise. Schema placement is unchanged (user domains in public, wrappers in eql_v3, blockers in eql_v3_internal); the surface stays self-contained (no eql_v2.*). Aliases are a SQL-surface concern only — no bindings, fixtures, or scalars:: matrix tests, so the pinned inventory baseline is untouched.

Testing

Every aliased family is tested against its alias at all levels, matrix-driven, on real encrypted fixtures:

  • Unit — catalog invariants (group_names, global uniqueness), render_cross_file both-directions + alias×alias symmetry, classifier, orphan-sweep (idempotent re-run + stray-artefact sweep), parity gate counting alias dirs.
  • Property (encrypted_domain::property::cross_name_routing) — for all 6 families: cross-name =/<> (HMAC) on _eq and </<=/>/>= (ORE/OPE) on _ord/_ord_ore/_ord_ope, both operand directions, over real eql_v3_<T>_doubles fixtures (equal-plaintext / distinct-ciphertext), with a self-guard that a meaningful pair exists. Wires real/double into the doubles fixtures so floats get equal-plaintext encrypted data.
  • Integration (v3_alias_interop_tests) — CHECK-identity + public-wrapper/internal-blocker split over all 6 pairs × both directions; unsupported-op raises for < @> <@ ||. v3_uninstall_tests adds a pg_operator teardown assertion.

tests/docker-compose.yml raises max_locks_per_transaction to 1024: the enlarged surface makes concurrent #[sqlx::test] install/uninstall transactions exhaust the default shared lock table (CI applies this compose file via mise run postgres:up). Single-connection DROP SCHEMA … CASCADE at the default 64 is unaffected, so there is no user-facing impact.

LOC breakdown

git diff --shortstat reports 112 files, 29,501 insertions / 63 deletions against the merge base (ecae8542), but ~96% is machine-produced, parity-gated SQL:

Bucket Insertions Share Nature
src/v3/scalars/<alias>/ (84 files) 20,796 70% Generated — per-alias domain surface (6 families × 14 files)
src/v3/scalars/**__**_cross.sql (6 files) 7,668 26% Generated — cross-name operator files (~1,278 lines each)
crates/ (Rust + minijinja templates) 551 2% Hand-written — catalog row, renderers, classifier, templates
tests/ (harness + suites) 405 1% Hand-written — property + integration cross-name tests
docs/ 80 <1% Docs

Summary:

  • 28,464 (96.5%) is committed generated SQL — CI-gated byte-for-byte by codegen:parity, not human-authored.
  • A further set of regenerated real/double doubles fixtures is gitignored, so it never appears in the diff.
  • The genuine hand-authored change is ~1,000 LOC across 22 files, single-sourced from the aliases catalog field + the eql-codegen renderers. Adding another alias is one array entry.

Review implication: the 90 generated files do not need line-by-line review — codegen:parity verifies them. Reviewer attention belongs on the 22 hand-written files: the catalog/codegen change (crates/eql-domains, crates/eql-codegen) and the cross-name test suites (tests/sqlx).

Review

Reviewed with CodeRabbit and four parallel dimension agents (architecture, reuse, type-safety, test-coverage). Findings addressed: multi-alias all-pairs fan-out; full ORE/OPE ordering coverage for all families; CREATE OPERATOR partial extraction (byte-identical); and the schema-qualification safety caveat in docs + CHANGELOG.md.

🤖 Generated with Claude Code

tobyhede added 16 commits July 8, 2026 12:04
…oss operators

Also raise test Postgres max_locks_per_transaction to 1024 so concurrent
install/uninstall #[sqlx::test] cases don't exhaust the shared lock table
over the enlarged surface.
…-in type)

Addresses CodeRabbit review: bare native spellings (int4, decimal, ...) resolve
to the pg_catalog built-in, not the encrypted public.<name> domain — a plaintext
footgun. Document schema-qualification prominently; fix the adding-an-alias
example to not imply integer already aliases 'int'.
…lti-alias safe)

Fixes the latent gap where a family with >1 alias would leave alias<->alias
comparisons un-shadowed (silently degrading to native jsonb). generate_all now
fans out over every unordered pair of group_names(); byte-identical for the
current single-alias-per-family catalog.
…milies

Every aliased ordered-scalar family (smallint/int2, integer/int4, bigint/int8,
real/float4, double/float8, numeric/decimal) now verifies cross-name = / <> (HMAC)
and < <= > >= on _ord/_ord_ore/_ord_ope (ORE/OPE), both operand directions, over
real encrypted doubles fixtures with an equal-plaintext/distinct-ciphertext
self-guard. Wires real/double into DOUBLES_TOKENS + the doubles loader so floats
get equal-plaintext encrypted fixtures too.
…pported ops

CHECK-identity and operator public/blocker split now iterate ALIAS_PAIRS (all 6
canonical/alias pairs) in both operand directions; unsupported cross-op raise
test covers <, @>, <@, || (not just <).
…urn-type compute; doc render_type

Extract the CREATE OPERATOR block into operators/create.sql.j2, included by both
operators.sql.j2 and cross.sql.j2 (byte-identical output, verified by parity).
Hoist dd_sig/returns into the blocker branch where it's used. Restore the
render_type doc comment.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

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

Run ID: 64770e02-bff3-4598-be14-04b4eb96e4bc

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 eql-v3-encrypted-domain-aliases

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

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

@freshtonic freshtonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

Adds Postgres-native spelling aliases (int2/int4/int8/float4/float8/decimal) as full standalone encrypted domains, generated from a new aliases field on the catalog's DomainFamily, plus cross-name operators over every unordered pair of group names so an alias and its canonical twin compare through the encrypted index term rather than falling through to native jsonb.

The codegen design is the strongest part of this PR. group_names() as the single fan-out site is the right seam; cross_name_operators() deriving the shadow set from Domain/Domain signatures (rather than a hardcoded symbol list) with a pinned assertion so a new operator fails loudly is exactly the "derive from data AND assert the result" pattern the module already uses; extract_arg correctly emits no cast when both operands are domains; blockers stay plpgsql/non-STRICT; and the R4 orphan-sweep keep-set fix is real and tested.

What I verified

I built release/cipherstash-encrypt.sql from this branch and exercised it against a stock postgres:17 (max_locks_per_transaction=64, max_connections=100).

Everything the PR claims about behaviour holds:

  • Transactional install succeeds at default lock settings. psql -1 -f release/cipherstash-encrypt.sql → exit 0. I expected the +84% object count (4,543 → 8,359 top-level CREATEs) to blow the shared lock table and contradict the "no user-facing impact" note on the compose bump. It does not. The bump really is test-harness-only. Good call flagging it in the PR body.
  • Cross-name equality routes HMAC in both directions. Two payloads with equal hm and distinct c compare t as int4_eq = integer_eq and as integer_eq = int4_eq.
  • Cross-name ordering binds the encrypted operator. pg_operator for < over (int8_ord, bigint_ord) has oprcode = eql_v3.lt.
  • All four unsupported cross ops raise on the _eq role (<, @>, <@, ||).
  • Uninstall is clean. DROP SCHEMA … CASCADE takes all 4,035 EQL operators to zero, which makes the new v3_uninstall_tests pg_operator assertion well-placed.

Blocking

1. The branch is 74 commits behind eql_v3, and CIP-3472 has invalidated the feature's premise

eql_v3 now prefixes every public column domain with eql_v3_ (PUBLIC_TYPNAME_PREFIX, crates/eql-domains/src/lib.rs:61; DomainFamily::domain_nameDomain::sql_typname). Today eql_v3 ships public.eql_v3_integer, not public.integer.

The rebase itself is mechanical — render_type_named calls domain_name(&d.full_name(name)), and post-rebase context::domain_name prepends the prefix, so aliases would come out as public.eql_v3_int4. The problem is semantic:

  • The headline benefit — "a schema can use whichever native name it already reaches for" — evaporates. public.eql_v3_int4 is neither native nor familiar, and public.eql_v3_integer is no harder to type.
  • The entire "⚠️ Always schema-qualify" argument in the PR body, in docs/reference/aliases.md, and in the CHANGELOG.md entry becomes false: eql_v3_int4 cannot shadow a built-in, because no built-in is named that.

Given the cost (~29.5k LOC, +84% installed objects, +540 operators in public), I think this needs a design decision before the rebase rather than after: does an eql_v3_int4 spelling still earn its keep alongside eql_v3_integer?

2. Changelog discipline: wrong file, no changeset, and a dead PR link

.changeset/ exists on eql_v3 but not at this branch's merge-base, so the branch predates the Changesets migration. Per CLAUDE.md, root CHANGELOG.md is now the frozen pre-3.0 archive and must not be hand-edited — the release changelog is assembled from .changeset/*.md. As it stands this PR edits the archive and ships no changeset, so the alias feature would not appear in the released changelog at all.

Separately, the entry links ([#370](https://github.com/cipherstash/encrypt-query-language/pull/370)). PR #370 does not exist (gh pr view 370 → "Could not resolve to a PullRequest"). This is #381. Moving the prose into a changeset fixes both problems at once — Changesets adds the PR link itself.

Correctness

3. docs/reference/aliases.md — the one safety-critical paragraph is wrong for double

The doc asserts:

every bare-family name in this set — int2, int4, … and their canonical spellings smallint, integer, bigint, real, double precision, numeric — is also a built-in PostgreSQL type name, resolved from pg_catalog, which always shadows the public schema.

Against the installed surface:

bare int4    ->  pg_catalog.int4   (typtype = b)   ✅ as documented
bare double  ->  public.double     (typtype = d)   ❌ the ENCRYPTED domain
SELECT count(*) FROM pg_type … WHERE nspname='pg_catalog' AND typname='double';  -->  0

double is not a pg_catalog type — the built-in is double precision. So bare double resolves through search_path to public.double, and the doc's warning is precisely inverted for that name: a user who writes CREATE TABLE t (x double) trusting this paragraph gets an encrypted domain, not a plaintext column.

The mechanism is also misdescribed for the rest. integer, smallint, bigint, real, decimal, numeric bind through the grammar (col_name_keyword → typename), not through pg_catalog shadowing public. The pg_catalog typnames are int2/int4/int8/float4/float8/numeric — i.e. mostly the aliases, not the canonical spellings.

The advice ("always schema-qualify") stays correct; the explanation and the double case do not. Note this section probably disappears under CIP-3472 anyway — but if any of it survives, it needs fixing.

4. Cross-family comparison still degrades silently, and aliases make the near-miss much easier to type

-- equal hm, distinct c
('…hm:a,c:x'::jsonb::public.int4_eq) = ('…hm:a,c:y'::jsonb::public.bigint_eq)   -->  f

EXPLAIN VERBOSE:
  ((…::int4_eq)::jsonb = (…::bigint_eq)::jsonb)     -- native jsonb =

That is exactly the silent-wrong-answer this feature exists to eliminate, one family over. It is pre-existing — the base branch has no cross operators at all — so this is not a regression, and I don't think it must block.

But it's worth weighing: before this PR the wrong pairing looked wrong (integer_eq vs bigint_eq are visibly different types). After it, the wrong pairing is int4_eq vs int8_eq — one character apart, both spelled "int", and adjacent in exactly the mental model the alias table invites. aliases.md covers this in one sentence ("any incidental behaviour is not part of the contract"), which understates an operator that returns false for equal plaintexts rather than raising.

Minimum: promote it to a ⚠️ with the concrete int4_eq = int8_eq example. Better, as a follow-up: the cross_name_operators() classifier is already signature-derived, so emitting cross-family blockers is the same machinery — though the pair count needs a look before committing to it.

Test quality

5. The R3 guard cannot fail for the reason it claims

crates/eql-codegen/src/generate.rs:

assert!(sql.contains("eql_v3_internal.") && sql.contains("RETURNS jsonb")); // the || blocker

Two independent contains over the whole file: it passes as long as some blocker exists and some function returns jsonb. If the || blocker regressed to RETURNS boolean this would still be green, which defeats the point of an R3 guard on signature-derived return types. Anchor it to the concat blocker's own signature:

assert!(sql.contains(
    "CREATE FUNCTION eql_v3_internal.\"||\"(a public.integer_eq, b public.int4_eq)\nRETURNS jsonb"
));

6. unsupported_cross_name_ops_raise can't distinguish the directions

The assertion is msg.contains("not supported"). The blocker's RAISE names only the left domain, so the two directions genuinely produce different messages, and the test asserts neither. Worth pinning the domain name per direction.

Relatedly, the message itself reads a little oddly for a cross operator: operator < is not supported for public.int4_eq when the real fact is that the pair has no ordering. Naming both operands would be clearer, and would give the test something direction-specific to assert.

7. alias_domains_exist_with_identical_check normalizes nothing

canon.map(|s| s.replace(canonical, "T")) == alias_c.map(|s| s.replace(alias, "T"))

The comment above it says the generated CHECK body carries no type name — which is true, so both replaces are no-ops and the test is really "the CHECK bodies are byte-identical". That's the stronger, clearer assertion; the .replace() calls imply a normalization that isn't happening and would be substring-fragile if a type name ever did appear. Also worth adding assert!(canon.is_some()) so a vanished canonical domain fails on its own terms.

Nits

  • 540 new operators land in public, not eql_v3. CREATE OPERATOR is emitted unqualified, so all 4,035 EQL operators are created in public (pre-existing, and DROP SCHEMA … CASCADE does clean them up — verified). Just noting the public operator footprint grew ~15% and nothing in the PR mentions it.
  • group_names() allocates a Vec per call and is called inside the generate_all loops. Irrelevant at this scale, but it could return an impl Iterator.
  • aliases.md isn't linked from any docs index that I can see.

Summary

The generator work is well-factored and the behaviour is correct — I confirmed the cross-name routing, the blockers, the uninstall teardown, and (against my own expectation) that the object-count growth doesn't break transactional installs at default lock settings.

What holds it up is not the code. The branch was written against a base that no longer exists: CIP-3472 renamed the public domains to eql_v3_*, which removes both the ergonomic reason for the aliases and the shadowing hazard the docs are built around. I'd resolve that question before spending a rebase on 29.5k lines of generated SQL. Items 2 and 3 are small and should be fixed regardless; 5–7 are cheap test hardening.

Base automatically changed from eql_v3 to main July 9, 2026 14:43

@freshtonic freshtonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation is excellent and I'm confident in it — requesting changes only on the release/changelog seam, which is a process fork rather than a code problem.

Implementation: verified clean

Regenerated the surfaces from this branch and checked the invariants mechanically (not by eye):

  • SQL surface regenerated → zero drift (16 type surfaces incl. all 6 aliases); Rust bindings regenerated → zero drift, and the aliases correctly produce no new bindings, matching the "purely a SQL-surface concern" design.
  • cargo test -p eql-domains -p eql-codegen94 + 98 pass, including the new alias/cross-file unit tests and the parity gate.
  • Footgun invariants hold on the real generated integer__int4_cross.sql: wrappers are LANGUAGE sql IMMUTABLE STRICT single-SELECT with no SET search_path (inlinable); blockers are LANGUAGE plpgsql and not STRICT; no CREATE CAST; alias domains are ... AS jsonb (no domain-over-domain); no opclass on a domain; no eql_v2 references.
  • Coverage is strong: v3_alias_interop_tests.rs (CHECK identity, wrapper/blocker split both directions, HMAC-not-jsonb routing, blockers raise), property/cross_name_routing.rs, and docs/reference/aliases.md is present (the CHANGELOG link resolves).

cross_name_operators() deriving the cross surface from the Domain/Domain signature set (with a pinned-symbol assertion) and the symmetric alias×alias handling are particularly nice.

Blocking: release-process reconciliation

  1. No changeset for a user-facing feature. Per the repo's release discipline (CLAUDE.md / .changeset/README.md), every releasable change needs a .changeset/*.md and CHANGELOG.md is not hand-edited — Changesets owns the version bump and the generated packages/eql/CHANGELOG.md. This PR adds no changeset, so as it stands the alias feature won't bump the version (this is a minor) and won't appear in the release notes. Please add a minor changeset.
  2. This revives the frozen root CHANGELOG.md. The PR reintroduces an ## [Unreleased] section (and rewrites the "how to read this file" header to describe [Unreleased] promotion) on a changelog that is currently the frozen pre-3.0 archive (its newest section is [2.3.1]; 3.x lives in packages/eql/CHANGELOG.md). Moving the project back to a hand-maintained root changelog is a real workflow decision that conflicts with the active Changesets model and with every other in-flight PR (e.g. #421). If that migration is intended, let's make it an explicit, standalone decision (and update CLAUDE.md); it shouldn't land implicitly inside a feature PR.
  3. CHANGELOG entry links to #370, not this PR (#381). Looks like a stale copy-paste — worth confirming/fixing to whichever PR actually merges.

None of this touches the code, which is ready. Once the changeset/changelog approach is settled I'll happily re-approve.

(Not run here: the DB-backed SQLx tests — they need a PG container + ZeroKMS creds, which CI has; the committed generated surface is the drift-authoritative check and it's clean.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants