feat(drizzle): EQL v3 adapter (text scalar)#513
Open
tobyhede wants to merge 8 commits into
Open
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 (2)
📝 WalkthroughWalkthroughIntroduces EQL v3 encrypted-column support for the Drizzle Postgres adapter. A new ChangesEQL v3 encrypted domain support
Sequence Diagram(s)sequenceDiagram
participant App
participant createProtectOperators
participant v3Dialect
participant eqlV3Type
participant registerColumnConfig
participant globalThis_map
participant PostgreSQL
App->>eqlV3Type: eqlV3Type('t_eq', { dataType: 'text', index: 'equality' })
eqlV3Type->>registerColumnConfig: register EncryptedColumnConfig
registerColumnConfig->>globalThis_map: set(Symbol.for key, config)
eqlV3Type-->>App: customType column with _protectConfig
App->>createProtectOperators: createProtectOperators(client, v3Dialect)
createProtectOperators-->>App: protectOps { eq, gt, ilike, inArray, asc, ... }
App->>createProtectOperators: ops.eq(column, 'alice')
createProtectOperators->>v3Dialect: equality('eq', column, encryptedValue)
v3Dialect-->>createProtectOperators: eql_v3.eq_term(col) = eql_v3.hmac_256(enc::jsonb)
createProtectOperators-->>App: SQL predicate
App->>PostgreSQL: SELECT WHERE eql_v3.eq_term(col) = eql_v3.hmac_256(...)
PostgreSQL-->>App: matched rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/drizzle/__tests__/v3/eql-v3-type.test.ts (1)
22-39: ⚡ Quick winAvoid asserting Drizzle private internals in this test.
These expectations depend on
config.customTypeParams, which is internal and brittle. Prefer asserting the SQL type through the public column API afterpgTableconstruction.Suggested refactor
- // biome-ignore lint/suspicious/noExplicitAny: reading Drizzle internals in test - expect((storage as any).config.customTypeParams.dataType()).toBe( - 'eql_v3.text', - ) - // biome-ignore lint/suspicious/noExplicitAny: reading Drizzle internals in test - expect((eqCol as any).config.customTypeParams.dataType()).toBe( - 'eql_v3.text_eq', - ) - // biome-ignore lint/suspicious/noExplicitAny: reading Drizzle internals in test - expect((matchCol as any).config.customTypeParams.dataType()).toBe( - 'eql_v3.text_match', - ) - // biome-ignore lint/suspicious/noExplicitAny: reading Drizzle internals in test - expect((ordCol as any).config.customTypeParams.dataType()).toBe( - 'eql_v3.text_ord', - ) + const t = pgTable('v3_types_sql', { + storage, + eqCol, + matchCol, + ordCol, + }) + expect(t.storage.getSQLType()).toBe('eql_v3.text') + expect(t.eqCol.getSQLType()).toBe('eql_v3.text_eq') + expect(t.matchCol.getSQLType()).toBe('eql_v3.text_match') + expect(t.ordCol.getSQLType()).toBe('eql_v3.text_ord')As per coding guidelines, "Prefer testing via public API; avoid reaching into private internals in tests".
🤖 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 `@packages/drizzle/__tests__/v3/eql-v3-type.test.ts` around lines 22 - 39, Replace the private Drizzle internals assertions in the test expectations for storage, eqCol, matchCol, and ordCol variables. Instead of accessing (variable as any).config.customTypeParams.dataType(), use the public column API method getSQLType() which provides the same information post-pgTable construction. Update all four expect() calls to use the getSQLType() method and remove the biome-ignore comments since you will no longer be accessing internal properties.Source: Coding guidelines
🤖 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 `@packages/drizzle/__tests__/eql-v3.test.ts`:
- Line 26: The SKIP_ORDER_BY constant at line 26 is hardcoded to true,
permanently disabling ORDER BY/MIN-MAX test coverage and preventing detection of
regressions in the ordering path. Replace the hardcoded true value with logic
that checks an environment flag (such as process.env.SKIP_ORDER_BY or a similar
CI capability flag) so these tests run by default but can be optionally skipped
when needed. Apply the same environment-based gating pattern to the related code
at lines 190-206 that also controls skip behavior based on this constant.
In `@packages/drizzle/__tests__/v3/extraction.test.ts`:
- Around line 40-57: The test uses a hardcoded column name 't_eq' which is
registered in a global map keyed by column name, creating the risk of false
positives if another test has already registered the same 't_eq' name. Replace
the column name 't_eq' with a unique identifier (such as 'v3_shared_t_eq' or
similar) throughout the test in three places: in the column definition passed to
eqlV3Type in the pgTable call, in the sharedMap.get() assertion, and in the
getEncryptedColumnConfig() call to ensure this test specifically validates its
own registration and fallback path without depending on prior test state.
In `@packages/drizzle/__tests__/v3/provisioning.test.ts`:
- Around line 34-44: The test in 'the v3 extractor functions were installed
(eq_term, ord_term, match_term)' currently only validates that at least 3 rows
are returned, which is insufficient because a single extractor function with
multiple overloads could satisfy that condition. Instead, extract the distinct
proname values from the fns result and explicitly assert that the set contains
all three required function names: eq_term, ord_term, and match_term. Replace
the toBeGreaterThanOrEqual(3) assertion with explicit checks that each of these
three names is present in the query results.
---
Nitpick comments:
In `@packages/drizzle/__tests__/v3/eql-v3-type.test.ts`:
- Around line 22-39: Replace the private Drizzle internals assertions in the
test expectations for storage, eqCol, matchCol, and ordCol variables. Instead of
accessing (variable as any).config.customTypeParams.dataType(), use the public
column API method getSQLType() which provides the same information post-pgTable
construction. Update all four expect() calls to use the getSQLType() method and
remove the biome-ignore comments since you will no longer be accessing internal
properties.
🪄 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: f90efbe7-2c43-47bc-82c6-996971a07fde
📒 Files selected for processing (25)
packages/drizzle/__tests__/eql-v3.test.tspackages/drizzle/__tests__/fixtures/cipherstash-encrypt-v3.sqlpackages/drizzle/__tests__/fixtures/eql-v3-seed-data.tspackages/drizzle/__tests__/test-utils.tspackages/drizzle/__tests__/v3/codec.test.tspackages/drizzle/__tests__/v3/detection.test.tspackages/drizzle/__tests__/v3/dialect.test.tspackages/drizzle/__tests__/v3/domain-map.test.tspackages/drizzle/__tests__/v3/eql-v3-type.test.tspackages/drizzle/__tests__/v3/exports.test.tspackages/drizzle/__tests__/v3/extraction.test.tspackages/drizzle/__tests__/v3/helpers/install-v3.tspackages/drizzle/__tests__/v3/operators-v3.test.tspackages/drizzle/__tests__/v3/provisioning.test.tspackages/drizzle/__tests__/v3/roundtrip-eq.test.tspackages/drizzle/package.jsonpackages/drizzle/scripts/refresh-eql-v3-sql.mdpackages/drizzle/src/pg/index.tspackages/drizzle/src/pg/operators.tspackages/drizzle/src/pg/sql-dialect.tspackages/drizzle/src/pg/v3/codec.tspackages/drizzle/src/pg/v3/domain-map.tspackages/drizzle/src/pg/v3/eql-v3-type.tspackages/drizzle/src/pg/v3/index.tspackages/drizzle/tsup.config.ts
| // DB-gated suite: skipped (not failed) without DATABASE_URL — see provisioning.test.ts. | ||
| const HAS_DB = !!process.env.DATABASE_URL | ||
|
|
||
| const SKIP_ORDER_BY = true // shared CI DB, mirrors drizzle.test.ts:61 |
There was a problem hiding this comment.
SKIP_ORDER_BY is hardcoded to true, so ORDER BY/MIN-MAX coverage never runs.
This permanently disables validation of the ordering path and can hide regressions. Gate skip behavior with an environment flag (or CI capability detection) so these tests execute by default when supported.
Suggested fix
-const SKIP_ORDER_BY = true // shared CI DB, mirrors drizzle.test.ts:61
+const SKIP_ORDER_BY = process.env.SKIP_ORDER_BY === '1'Also applies to: 190-206
🤖 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 `@packages/drizzle/__tests__/eql-v3.test.ts` at line 26, The SKIP_ORDER_BY
constant at line 26 is hardcoded to true, permanently disabling ORDER BY/MIN-MAX
test coverage and preventing detection of regressions in the ordering path.
Replace the hardcoded true value with logic that checks an environment flag
(such as process.env.SKIP_ORDER_BY or a similar CI capability flag) so these
tests run by default but can be optionally skipped when needed. Apply the same
environment-based gating pattern to the related code at lines 190-206 that also
controls skip behavior based on this constant.
Extract per-version SQL emission into a SqlDialect seam so the operator factories share all lazy/batch plumbing and differ only by dialect. v2 behaviour is unchanged; v3 compares extracted index terms (eq_term/ord_term/ match_term vs the hmac_256/ore/bloom jsonb helpers) because the v3 domain CHECKs reject index-only search terms. All equality routes through the seam, including inArray/notInArray set membership.
Pure (dataType, index) -> eql_v3 domain mapping with a scalar-keyed capability table (invalid tuples cannot type-check), plus a plain-jsonb wire codec. null/undefined bind as SQL NULL (not the JSONB null literal), which the v3 domain CHECK (jsonb_typeof = 'object') accepts.
Single-capability eqlV3Type column builder; shared isEncryptedSqlName predicate covering both v2 and v3 domains; v3 columns survive extractProtectSchema. The column-config registry is anchored on a global Symbol so the ./pg and ./pg/v3 bundles (separate, un-split CJS outputs) share one map — otherwise mixed-import CJS consumers would see no encrypted columns.
Expose the v3 adapter under ./pg/v3 via a keyed tsup entry (avoids the index.ts basename collision) and a package.json exports map. On this subpath createProtectOperators (and the createProtectOperatorsV3 alias) pre-binds the v3 dialect, so v3 consumers can't reach the v2-defaulted factory and silently emit v2 SQL that fails the v3 domain CHECKs.
Assert v3 operators emit encrypted params through v3Dialect (eq/ne, gt/gte/lt/lte, between/notBetween, match, inArray/notInArray, asc/desc) and fall back to native SQL on unencrypted columns; cover lazy combination under and().
Check in the self-contained eql_v3 installer (text scalar, EQL 035952e) used by the integration suite, and document the refresh process. Mark the fixture linguist-vendored (codegen output) so it's excluded from language stats and collapsed in diffs.
…ain matrix) DB-gated suites (describe.skipIf without DATABASE_URL): advisory-lock installer, text_eq round-trip, and the text domain matrix with an in-memory oracle and negative assertions (domain CHECK, blocker RAISE, wrong-domain).
tobyhede
force-pushed
the
eql-v3-drizzle-adapter
branch
from
da1c57a to
d13a389
Compare
- extraction.test.ts: use a unique column key + pre-delete from the global registry so the cross-bundle test can't pass on another test's stale state - provisioning.test.ts: assert the distinct extractor-name set instead of a bare row count (>=3 could pass with one overloaded name)
Contributor
Author
Fixes Applied SuccessfullyApplied 2 of 3 CodeRabbit feedback items based on local review. Files modified:
Deferred (1):
Commit: The latest autofix changes are on the |
coderdan
reviewed
Jun 17, 2026
| * is a type-shape extension (new key + its valid index subset), not a row append | ||
| * that lets invalid (scalar, capability) tuples type-check (spec §5.1, §11 item 5). | ||
| */ | ||
| const DOMAINS: Record< |
Contributor
There was a problem hiding this comment.
Shouldn't this be using the EQL TS types from eql-types?
coderdan
reviewed
Jun 17, 2026
coderdan
left a comment
coderdan
left a comment
Contributor
There was a problem hiding this comment.
I'm a bit confused by this PR because it takes quite a different approach to what was discussed. I thought the whole idea of the eql-types crate and TS bindings was to use those types in consumers to avoid duplication, drift and bugs due to inconsistency.
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
2 participants
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
Adds an EQL v3 adapter to the Drizzle Postgres integration, alongside the existing v2 support. v3 columns are
CREATE DOMAIN … AS jsonbtypes (eql_v3.text{,_eq,_match,_ord}); queries compare extracted index terms (eq_term/ord_term/match_termvs thehmac_256/ore_block_u64_8_256/bloom_filterjsonb helpers) rather than coercing search terms into the domain — which would fail the domain CHECK (SQLSTATE 23514). This milestone covers the text scalar.The change is structured as 7 cohesive, layered commits (each builds + tests green):
refactor: SQL-dialect seam with v2 + v3 emitters— extracts aSqlDialectseam so operator factories share all lazy/batch plumbing and differ only by dialect. v2 behaviour unchanged.feat: eql_v3 domain mapping + jsonb wire codec— pure(dataType, index) -> domainmapping; plain-jsonb codec (null/undefined bind as SQL NULL).feat: eqlV3Type column builder + encrypted-column detection— single-capability column builder; sharedisEncryptedSqlName; column-config registry anchored on a globalSymbolso the./pgand./pg/v3bundles share one map.feat: ./pg/v3 export subpath + v3-bound operators— keyed tsup entry + exports map;createProtectOperatorson the v3 subpath pre-binds the v3 dialect (no v2-default footgun).test: v3 operator param-emission unit tests— eq/ne, gt/gte/lt/lte, between/notBetween, match, inArray/notInArray, asc/desc, lazyand()combine, native fallback.chore: vendor eql_v3 SQL fixture + refresh doc— checked-ineql_v3installer (EQL035952e) + refresh process.test: v3 DB integration suite— provisioning, text_eq round-trip, text domain matrix with an in-memory oracle and negative assertions.Testing
tsc --noEmitclean · biome clean on all touched files.describe.skipIfwhenDATABASE_URLis unset (run them against a Postgres with the v3 SQL installed).operators.test.ts(31 tests) unchanged — no regression.Notes for reviewers
packages/drizzle/__tests__/fixtures/cipherstash-encrypt-v3.sql(~16k lines) is a vendored, codegen-produced EQL artifact, not hand-written — seescripts/refresh-eql-v3-sql.md. It dominates the diff line count.dist/is a possible follow-up if desired.🤖 Generated with Claude Code
Summary by CodeRabbit
./pg/v3package subpath for v3-specific helpers.null/undefinedto SQLNULLand round-trips plain JSON strings/objects.