-
-
Notifications
You must be signed in to change notification settings - Fork 17
fix(orm): add special treatment to enum arrays for postgres db #577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- For input, the string array needs to be casted with "Enum"[] - For output, the raw pg array string needs to be parsed back to a proper JS string array fixes #576
📝 WalkthroughWalkthroughAdds PostgreSQL enum-array support: new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
|
Claude finished @ymc9's task —— View job PR Review for Enum Array PostgreSQL SupportProgress Checklist
Review SummaryThis PR successfully addresses issue #576 by implementing proper enum array handling for PostgreSQL. The implementation is well-designed and follows PostgreSQL best practices. ✅ Strengths1. Input Transformation Logic (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds special handling for enum arrays in PostgreSQL databases. The implementation adds type casting for enum array inputs (using CAST(ARRAY[...] AS "EnumType"[])), parsing for enum array outputs (converting PostgreSQL's array string format to JavaScript arrays), and schema-qualified enum type references to support custom PostgreSQL schemas.
Key changes:
- Added
postgres-arraydependency for parsing PostgreSQL array strings - Modified dialect transform methods to accept an
arrayparameter indicating whether the field is an array - Added input transformation to cast enum arrays with proper schema qualification
- Added output transformation to parse PostgreSQL enum array strings back to JavaScript arrays
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/regression/test/issue-576.test.ts | Comprehensive test coverage for enum arrays including basic fields, JSON fields, and custom schemas |
| tests/e2e/orm/client-api/pg-custom-schema.test.ts | Modified BarRole enum schema to test cross-schema references |
| packages/orm/src/client/crud/dialects/postgresql.ts | Added enum array casting for input and parsing for output, plus schema qualification logic |
| packages/orm/src/client/crud/dialects/base-dialect.ts | Added array parameter to transformOutput signature |
| packages/orm/src/client/crud/dialects/sqlite.ts | Propagated array parameter in transformOutput implementation |
| packages/orm/src/client/result-processor.ts | Updated to pass array parameter when calling dialect transform methods |
| packages/orm/package.json | Added postgres-array dependency |
| pnpm-lock.yaml | Updated lock file with postgres-array@3.0.4 and related dependency updates |
| packages/testtools/src/client.ts | Changed stdio from 'ignore' to 'inherit' for Prisma DB push output |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/orm/src/client/crud/dialects/postgresql.ts:
- Around line 113-137: The createSchemaQualifiedEnumType function currently only
looks for a named 'map' arg on the @@schema attribute which misses positional
usage like @@schema('mySchema'); update createSchemaQualifiedEnumType to check
for a positional arg when no named map is found (e.g., use schemaAttr.args?.[0]
if mapArg is undefined, verify arg.value.kind === 'literal' and use
arg.value.value as the schema name) and then build qualified =
`"${schemaName}"."${type}"`; apply the same change to the analogous
schema-handling code in name-mapper (the model schema mapping logic around line
207) so positional @@schema arguments are handled there too.
🧹 Nitpick comments (1)
packages/orm/src/client/crud/dialects/postgresql.ts (1)
209-220: Consider adding logging for array parsing failures.The try-catch block silently falls through when
parsePostgresArrayfails (Line 216), returning the unparsed value. While this provides graceful degradation, it may make debugging difficult if enum arrays are not being parsed correctly.💡 Optional improvement: Add debug logging for parse failures
private transformOutputEnum(value: unknown, array: boolean) { if (array && typeof value === 'string') { try { // postgres returns enum arrays as `{"val 1",val2}` strings, parse them back // to string arrays here return parsePostgresArray(value); - } catch { + } catch (e) { // fall through - return as-is if parsing fails + if (this.options.log) { + console.warn('Failed to parse PostgreSQL enum array:', value, e); + } } } return value; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/orm/package.jsonpackages/orm/src/client/crud/dialects/base-dialect.tspackages/orm/src/client/crud/dialects/postgresql.tspackages/orm/src/client/crud/dialects/sqlite.tspackages/orm/src/client/result-processor.tspackages/testtools/src/client.tstests/e2e/orm/client-api/pg-custom-schema.test.tstests/regression/test/issue-576.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/client-api/pg-custom-schema.test.ts
🧠 Learnings (8)
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/crud/dialects/sqlite.tspackages/orm/src/client/crud/dialects/postgresql.tspackages/orm/src/client/crud/dialects/base-dialect.tspackages/testtools/src/client.tspackages/orm/src/client/result-processor.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
packages/orm/src/client/crud/dialects/postgresql.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/package.jsontests/e2e/orm/client-api/pg-custom-schema.test.ts
📚 Learning: 2025-10-21T16:04:56.292Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/crud/dialects/base-dialect.ts:745-747
Timestamp: 2025-10-21T16:04:56.292Z
Learning: In packages/runtime/src/client/crud/dialects/base-dialect.ts, it's intentional that buildCursorFilter applies default ordering (via makeDefaultOrderBy fallback) while buildOrderBy does not. This ensures cursor-based pagination always has stable ordering for correctness, while regular queries remain unordered unless explicitly specified. This design is to be consistent with Prisma's pagination requirements.
Applied to files:
packages/orm/src/client/crud/dialects/base-dialect.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Database migrations should use Prisma CLI under the hood via ZenStack commands
Applied to files:
packages/testtools/src/client.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/regression/test/issue-576.test.tstests/e2e/orm/client-api/pg-custom-schema.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/regression/test/issue-576.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/regression/test/issue-576.test.tstests/e2e/orm/client-api/pg-custom-schema.test.ts
🧬 Code graph analysis (4)
packages/orm/src/client/crud/dialects/sqlite.ts (1)
packages/schema/src/schema.ts (1)
BuiltinType(89-99)
packages/orm/src/client/crud/dialects/postgresql.ts (2)
packages/orm/src/client/query-utils.ts (2)
isEnum(193-195)getEnum(197-199)packages/schema/src/schema.ts (1)
BuiltinType(89-99)
packages/orm/src/client/crud/dialects/base-dialect.ts (2)
packages/schema/src/schema.ts (1)
BuiltinType(89-99)packages/plugins/policy/src/expression-transformer.ts (1)
_array(164-166)
tests/regression/test/issue-576.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(101-248)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: claude-review
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (13)
packages/testtools/src/client.ts (1)
206-209: LGTM! Improved test debugging experience.Changing
stdiofrom'ignore'to'inherit'makes Prisma output visible during test execution, which will help with debugging database push issues.tests/e2e/orm/client-api/pg-custom-schema.test.ts (1)
159-163: Enhanced test coverage for cross-schema enum usage.Changing the
BarRoleenum schema to'mySchema'while keeping theBarmodel in'public'creates a cross-schema enum reference scenario, which is valuable for testing PostgreSQL's handling of enum arrays across schemas.Based on coding guidelines: E2E tests should validate real-world schema compatibility with established projects.
packages/orm/src/client/crud/dialects/base-dialect.ts (1)
51-53: LGTM! Array-context parameter added to support enum array transformations.The additional
_arrayboolean parameter enables dialect-specific handling of array types, particularly for PostgreSQL enum arrays. The underscore prefix correctly indicates this parameter is unused in the base implementation but will be utilized by subclass overrides.packages/orm/src/client/result-processor.ts (2)
52-52: LGTM! Correctly passes array context for delegate field transformation.The
falseparameter correctly indicates that delegate joined fields are JSON objects, not arrays.
93-101: LGTM! Array context correctly propagated through field value processing.The
!!fieldDef.arrayparameter ensures that:
- Array elements (line 96) are transformed with array-context awareness
- Scalar values (line 99) are transformed according to their actual array status
This enables proper handling of PostgreSQL enum arrays during output transformation.
packages/orm/package.json (1)
93-93: No issues found with the postgres-array dependency.The version
^3.0.4is correct (latest published version as of Mar 6, 2025) and has no known security vulnerabilities. Continue to monitor with regularnpm auditand dependency scanning tools.packages/orm/src/client/crud/dialects/sqlite.ts (2)
70-70: LGTM! Signature updated to support array context.The addition of the
arrayparameter correctly aligns with the base class signature change and enables array-aware transformation throughout the dialect hierarchy.
84-84: LGTM! Array parameter correctly propagated to base class.The
arrayparameter is properly threaded through to the base class, ensuring array context is preserved for all unhandled type branches.packages/orm/src/client/crud/dialects/postgresql.ts (4)
12-12: LGTM! Required imports added for enum array support.The new imports (
parsePostgresArray,getEnum,isEnum) are necessary for the enum array handling functionality and are correctly utilized throughout the implementation.Also applies to: 24-24, 26-26
77-86: LGTM! Enum array input properly cast for PostgreSQL.The implementation correctly casts string arrays to PostgreSQL enum arrays using
CAST(ARRAY[...] AS "EnumType"[])syntax. Each element is recursively transformed, and schema-qualified enum types are properly resolved.
139-139: LGTM! Signature updated for array-aware output transformation.The addition of the
arrayparameter enables proper handling of enum arrays in output transformation.
148-152: LGTM! Enum types correctly routed to specialized output transformation.The
.when()clause properly identifies enum types and routes them totransformOutputEnum, ensuring array context is preserved throughout the transformation chain.tests/regression/test/issue-576.test.ts (1)
1-158: Test 5 will fail: schema attribute handling bug in PostgreSQL dialect code.The test suite has excellent coverage, but test 5 (line 124-157) will fail due to a bug in schema attribute parsing. The
@@schema('mySchema')attribute uses a positional argument, but the PostgreSQL dialect code inpackages/orm/src/client/crud/dialects/postgresql.ts(lines 122-128) andpackages/orm/src/client/executor/name-mapper.tsincorrectly searches for a namedmapargument that doesn't exist.When
@@schemais used, the code fails to extract the schema name and falls back to the default 'public' schema. This causes:
- Test 5 to create the enum in 'public' instead of 'mySchema'
- The test assertions to fail
- Real-world usage of custom schemas to be broken
Fix the schema attribute parsing in both files to access the positional argument directly (e.g.,
schemaAttr.args?.[0]?.value?.value) instead of searching for a non-existent namedmapargument.⛔ Skipped due to learnings
Learnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projectsLearnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
|
Claude finished @ymc9's task —— View job PR Review for Enum Array PostgreSQL SupportProgress Checklist
Review SummaryThis PR successfully addresses issue #576 by implementing proper enum array handling for PostgreSQL. The implementation is well-designed and follows PostgreSQL best practices. ✅ Strengths1. Input Transformation Logic (
|
|
Claude finished @ymc9's task —— View job PR Review for Enum Array PostgreSQL SupportProgress Checklist
Review SummaryThis PR successfully addresses issue #576 by implementing proper enum array handling for PostgreSQL. The implementation is well-designed and follows PostgreSQL best practices. ✅ Strengths1. Input Transformation Logic (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/testtools/src/client.ts (1)
206-209: LGTM! Helpful debugging enhancement.The conditional stdio output is a useful improvement that makes Prisma db push output visible during debugging, consistent with the debug logging on lines 159-161.
Minor optional suggestion: For consistency with line 158, consider using
options?.debuginstead ofoptions.debug:♻️ Optional consistency refinement
- stdio: options.debug ? 'inherit' : 'ignore', + stdio: options?.debug ? 'inherit' : 'ignore',
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/testtools/src/client.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Database migrations should use Prisma CLI under the hood via ZenStack commands
Applied to files:
packages/testtools/src/client.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/testtools/src/client.ts
🧬 Code graph analysis (1)
packages/testtools/src/client.ts (3)
packages/orm/src/client/crud/validator/index.ts (1)
options(69-71)packages/orm/src/client/crud/operations/base.ts (1)
options(104-106)packages/orm/src/client/executor/zenstack-query-executor.ts (1)
options(81-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: claude-review
|
Claude finished @ymc9's task —— View job PR Review for Enum Array PostgreSQL SupportProgress Checklist
Review SummaryThis PR successfully addresses issue #576 by implementing proper enum array handling for PostgreSQL. The implementation is well-designed and follows PostgreSQL best practices. ✅ Strengths1. Input Transformation Logic (
|
fixes #576
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.