Add EQL v3 Drizzle support#565
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a new EQL v3 Drizzle integration under ChangesEQL v3 Drizzle Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant createEncryptionOperatorsV3
participant EncryptionClient
participant v3Dialect
participant Postgres
App->>createEncryptionOperatorsV3: eq(column, value)
createEncryptionOperatorsV3->>createEncryptionOperatorsV3: resolve column context and requireIndex
createEncryptionOperatorsV3->>EncryptionClient: encryptQuery(value, queryType)
EncryptionClient-->>createEncryptionOperatorsV3: encrypted term
createEncryptionOperatorsV3->>v3Dialect: equality(term)
v3Dialect-->>createEncryptionOperatorsV3: SQL fragment
createEncryptionOperatorsV3-->>App: SQL
App->>Postgres: execute query with SQL
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🦋 Changeset detectedLatest commit: 338552a The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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 (3)
packages/stack/__tests__/drizzle-v3/column.test.ts (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest reaches into Drizzle's internal column config instead of the public API.
(col as any).config.customTypeParams.dataType()asserts on an internal Drizzle builder shape rather than exercising a public entry point (e.g.,col.getSQLType(), which is Drizzle's documented API for the emitted SQL domain type). This couples the test to internalcustomTypeParams/configshape called out as fragile incolumn.ts.
As per coding guidelines, "prefer testing through the public API rather than private internals."♻️ Proposed fix using public API
it('sets dataType() to the concrete eql_v3 domain', () => { const col = makeEqlV3Column(v3Types.Int4Ord('age')) - // biome-ignore lint/suspicious/noExplicitAny: reading drizzle internals in test - expect((col as any).config.customTypeParams.dataType()).toBe( - 'eql_v3.int4_ord', - ) + expect((col as unknown as { getSQLType(): string }).getSQLType()).toBe( + 'eql_v3.int4_ord', + ) })🤖 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/stack/__tests__/drizzle-v3/column.test.ts` around lines 11 - 18, The `makeEqlV3Column` test is asserting against Drizzle internals via `(col as any).config.customTypeParams.dataType()`, so update the test to verify the emitted SQL domain through the public `getSQLType()` API on the column returned by `makeEqlV3Column` instead. Keep the test focused on the same `makeEqlV3Column`/`v3Types.Int4Ord` path, but remove the `any` cast and internal `config` access so it only depends on documented behavior.Source: Coding guidelines
packages/stack/src/eql/v3/drizzle/operators.ts (2)
188-200: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnbounded concurrent encryption calls for large
inArray/notInArrayinputs.Each element triggers its own
equality()→encryptTerm()round trip, and all are fired concurrently viaPromise.allwith no batching or concurrency cap. A largevaluesarray could spike load on the encryption/KMS backend.Consider chunking or capping concurrency (e.g. via
p-limitor manual batching) for large arrays.🤖 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/stack/src/eql/v3/drizzle/operators.ts` around lines 188 - 200, The inArrayOp helper currently fans out one equality() call per value through Promise.all, which can overwhelm the encryption/KMS path for large arrays. Update inArrayOp to limit concurrency by batching or capping parallel equality() work (for example with a small worker pool or chunked awaits) while preserving the existing negate/or/and behavior and the empty-array true/false handling.
49-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider Drizzle's
is()guard instead of blindanycasts for column internals.
drizzleTableOf/resolveContextreach into(column as any).table/.name. Drizzle-orm exposesis(value, Column)specifically to narrow such values safely withoutany, which would also guard against non-ColumnSQLWrapperinputs being misread.🤖 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/stack/src/eql/v3/drizzle/operators.ts` around lines 49 - 56, The `drizzleTableOf` and `resolveContext` helpers are reading `.table` and `.name` via `any` casts on `SQLWrapper`, which should be replaced with Drizzle’s `is()` guard for `Column` instead. Update these helpers to first narrow the input with `is(value, Column)` and only then access column internals, falling back safely for non-Column wrappers. Keep the changes localized to the `drizzleTableOf` and `resolveContext` logic so the context resolution remains type-safe without relying on blind casts.
🤖 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 `@docs/superpowers/specs/2026-07-06-eql-v3-drizzle-concrete-types-design.md`:
- Line 113: The fenced example blocks in this spec are unlabeled, which triggers
docs lint and hurts readability. Update both fenced blocks in the affected
sections to include a language tag using the existing markdown fences in this
document, and keep the content otherwise unchanged. Use the surrounding
fenced-example markup in the spec to locate the two bare fences that need the
same labeling fix.
In `@packages/stack/src/eql/v3/drizzle/operators.ts`:
- Line 47: The table cache in operators.ts is keyed by table name, which can
collide for pgSchema() tables that share names across schemas and merge
unrelated entries via the 'unknown' fallback. Update the cache used by the v3
operator logic to key by the table object itself, replacing the current
Map<string, AnyV3Table> with a WeakMap<PgTable, AnyV3Table>, and adjust the
lookup/insert logic in the table caching path so the AnyV3Table metadata stays
attached to the correct table instance.
In `@packages/stack/src/eql/v3/drizzle/schema-extraction.ts`:
- Around line 20-25: In schema-extraction.ts, the column identifier passed into
getEqlV3Column() is using the JS property key instead of the Drizzle column
name, which can rebuild v3 columns under the wrong name in the fallback path.
Update the loop that populates columns to prefer column.name when available and
only fall back to property if no Drizzle name exists, keeping the rest of the
logic in getEqlV3Column() unchanged.
---
Nitpick comments:
In `@packages/stack/__tests__/drizzle-v3/column.test.ts`:
- Around line 11-18: The `makeEqlV3Column` test is asserting against Drizzle
internals via `(col as any).config.customTypeParams.dataType()`, so update the
test to verify the emitted SQL domain through the public `getSQLType()` API on
the column returned by `makeEqlV3Column` instead. Keep the test focused on the
same `makeEqlV3Column`/`v3Types.Int4Ord` path, but remove the `any` cast and
internal `config` access so it only depends on documented behavior.
In `@packages/stack/src/eql/v3/drizzle/operators.ts`:
- Around line 188-200: The inArrayOp helper currently fans out one equality()
call per value through Promise.all, which can overwhelm the encryption/KMS path
for large arrays. Update inArrayOp to limit concurrency by batching or capping
parallel equality() work (for example with a small worker pool or chunked
awaits) while preserving the existing negate/or/and behavior and the empty-array
true/false handling.
- Around line 49-56: The `drizzleTableOf` and `resolveContext` helpers are
reading `.table` and `.name` via `any` casts on `SQLWrapper`, which should be
replaced with Drizzle’s `is()` guard for `Column` instead. Update these helpers
to first narrow the input with `is(value, Column)` and only then access column
internals, falling back safely for non-Column wrappers. Keep the changes
localized to the `drizzleTableOf` and `resolveContext` logic so the context
resolution remains type-safe without relying on blind casts.
🪄 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: c223fed7-0a1c-4838-90db-88844e2a9f9f
📒 Files selected for processing (20)
.changeset/eql-v3-drizzle.mddocs/superpowers/specs/2026-07-06-eql-v3-drizzle-concrete-types-design.mdpackages/stack/__tests__/drizzle-v3/codec.test.tspackages/stack/__tests__/drizzle-v3/column.test.tspackages/stack/__tests__/drizzle-v3/exports.test.tspackages/stack/__tests__/drizzle-v3/operators-live-pg.test.tspackages/stack/__tests__/drizzle-v3/operators.test.tspackages/stack/__tests__/drizzle-v3/schema-extraction.test.tspackages/stack/__tests__/drizzle-v3/sql-dialect.test.tspackages/stack/__tests__/drizzle-v3/types.test-d.tspackages/stack/__tests__/drizzle-v3/types.test.tspackages/stack/package.jsonpackages/stack/src/eql/v3/drizzle/codec.tspackages/stack/src/eql/v3/drizzle/column.tspackages/stack/src/eql/v3/drizzle/index.tspackages/stack/src/eql/v3/drizzle/operators.tspackages/stack/src/eql/v3/drizzle/schema-extraction.tspackages/stack/src/eql/v3/drizzle/sql-dialect.tspackages/stack/src/eql/v3/drizzle/types.tspackages/stack/tsup.config.ts
|
|
||
| ## 4. Architecture & location | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the fenced examples.
These bare fences trip MD040 in docs lint and are harder to render/read. Add a language tag such as text to both blocks.
♻️ Proposed fix
-```
+```text-```
+```textAlso applies to: 227-227
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/specs/2026-07-06-eql-v3-drizzle-concrete-types-design.md` at
line 113, The fenced example blocks in this spec are unlabeled, which triggers
docs lint and hurts readability. Update both fenced blocks in the affected
sections to include a language tag using the existing markdown fences in this
document, and keep the content otherwise unchanged. Use the surrounding
fenced-example markup in the spec to locate the two bare fences that need the
same labeling fix.
Source: Linters/SAST tools
| } | ||
|
|
||
| export function createEncryptionOperatorsV3(client: EncryptionClient) { | ||
| const tableCache = new Map<string, AnyV3Table>() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does drizzle-orm allow two pgTable() definitions with the same table name in different pgSchema() namespaces?
💡 Result:
Yes, Drizzle ORM allows you to define tables with the same name in different PostgreSQL schemas [1][2][3]. While earlier versions had limitations regarding relational queries when tables shared the same name across different schemas [4], these issues were addressed in drizzle-orm@0.31.4 and subsequent releases [4][1]. To define tables with the same name in different schemas, you use the pgSchema() function to create a schema namespace, and then call the .table() method on that schema object [2][3]. Example usage: import { pgSchema, pgTable, serial, text } from 'drizzle-orm/pg-core'; // Table in the public schema export const users = pgTable('users', { id: serial('id').primaryKey, email: text('email').notNull, }); // Table with the same name in a custom 'auth' schema export const authSchema = pgSchema('auth'); export const authUsers = authSchema.table('users', { id: serial('id').primaryKey, email: text('email').notNull, }); In this example, Drizzle treats these as distinct entities: public.users and auth.users [2][3]. Note that you cannot use pgSchema('public') to define tables in the default schema; you must use the standard pgTable() function for that [2].
Citations:
- 1: fix(Relational queries): support tables sharing the same name across schemas drizzle-team/drizzle-orm#1965
- 2: https://drizzle-team-drizzle-orm.mintlify.app/api/postgresql/table
- 3: https://drizzle-team-drizzle-orm.mintlify.app/core/schema-declaration
- 4: [BUG]: Drizzle Queries not supporting tables with same name across schemas drizzle-team/drizzle-orm#1964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file outline ---'
ast-grep outline packages/stack/src/eql/v3/drizzle/operators.ts --view expanded || true
echo '--- relevant lines ---'
sed -n '1,170p' packages/stack/src/eql/v3/drizzle/operators.ts | cat -n
echo '--- search for tableCache and resolveContext usage ---'
rg -n "tableCache|drizzleTableOf|resolveContext|extractEncryptionSchemaV3|getEqlV3Column" packages/stack/src/eql/v3/drizzle/operators.tsRepository: cipherstash/stack
Length of output: 7718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant Drizzle schema identity docs / code references in repo ---'
rg -n "pgSchema\\(|schema\\)|getTableConfig|drizzle:Name|drizzle:Schema|AnyV3Table|extractEncryptionSchemaV3" packages -g '!**/node_modules/**' || trueRepository: cipherstash/stack
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- lines around cache and resolution ---'
sed -n '1,140p' packages/stack/src/eql/v3/drizzle/operators.ts | cat -n
echo '--- any other cache keyed by table name in this file ---'
rg -n "Map<string, AnyV3Table>|WeakMap|Symbol.for\\('drizzle:Name'\\)|extractEncryptionSchemaV3\\(" packages/stack/src/eql/v3/drizzle/operators.tsRepository: cipherstash/stack
Length of output: 5613
Key the table cache by table object, not name. pgSchema() tables can share a name across schemas, and the 'unknown' fallback collapses unrelated tables into one slot. That can reuse the wrong AnyV3Table and apply encryption/index metadata to the wrong table; use WeakMap<PgTable, AnyV3Table> instead.
🤖 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/stack/src/eql/v3/drizzle/operators.ts` at line 47, The table cache
in operators.ts is keyed by table name, which can collide for pgSchema() tables
that share names across schemas and merge unrelated entries via the 'unknown'
fallback. Update the cache used by the v3 operator logic to key by the table
object itself, replacing the current Map<string, AnyV3Table> with a
WeakMap<PgTable, AnyV3Table>, and adjust the lookup/insert logic in the table
caching path so the AnyV3Table metadata stays attached to the correct table
instance.
#565 ships the sibling integration but on the protect-ffi 0.26 / pre-rename-domain bundle line, while main (this module's base) is 0.27 + SQL-standard names with term constructors in eql_v3_internal and scalar encryptQuery unsupported — so its exact SQL and term-operand path do not transfer. Adopt its dialect shape instead: gating on build().indexes, equality-via-ORE fallback, encrypted ORDER BY (now in scope as a fragment builder), whereIn with bounded operand-encryption concurrency, and the no-silent-fallback error convention. Free-text match pinned to the two-arg eql_v3.contains form; sequencing no longer blocks on #565.
1 participant
Summary
@cipherstash/stack/eql/v3/drizzlewith concrete v3 Drizzle column types, schema extraction, and capability-checked operators.isEqlV3Columnuse DrizzlegetSQLType()API.Usage Example
Test Plan
cd packages/stack && pnpm biome check --write src/eql/v3/drizzle __tests__/drizzle-v3cd packages/stack && pnpm vitest run __tests__/drizzle-v3cd packages/stack && pnpm test:typescd packages/stack && pnpm buildNotes
DATABASE_URLand CipherStashCS_*credentials are present.Summary by CodeRabbit
New Features
Bug Fixes