feat(schema): support table-level constraints with array syntax in Drizzle v1#4065
Open
kobayashiyabako16g wants to merge 3 commits intoliam-hq:mainfrom
Open
feat(schema): support table-level constraints with array syntax in Drizzle v1#4065kobayashiyabako16g wants to merge 3 commits intoliam-hq:mainfrom
kobayashiyabako16g wants to merge 3 commits intoliam-hq:mainfrom
Conversation
…izzle v1 Add support for Drizzle ORM v1's `(table) => [...]` array syntax for table-level constraints in both PostgreSQL and MySQL parsers: - foreignKey(): supports custom name, composite columns, onDelete/onUpdate - check(): ported from MySQL implementation to PostgreSQL - unique(): ported from MySQL implementation to PostgreSQL - index() / uniqueIndex(): array syntax parity with object syntax - primaryKey() (composite): supports optional custom name via `name` option
kobayashiyabako16g
requested review from
NoritakaIkeda,
Copilot and
sasamuku
and removed request for
a team
|
Unexpected error on running flow
|
||||||||||||
Contributor
📝 WalkthroughWalkthroughAdds parsing, AST helpers, types, converters, and tests to support Drizzle v1 table-level constraints (foreign keys, composite primary keys, indexes, unique/check) for MySQL and PostgreSQL; converts parsed definitions into internal constraint objects. Changes
Sequence DiagramsequenceDiagram
participant Parser as Table Parser
participant AST as AST Utils
participant Converter as Converter
participant Store as Constraints Map
Parser->>AST: detect foreignKey() / other calls
AST-->>Parser: config object, column names, table+columns, sql template
Parser-->>Parser: build DrizzleForeignKeyDefinition / Constraint defs
Parser->>Converter: pass DrizzleTableDefinition with foreignKeys/constraints
Converter->>Converter: resolve target table names, map columns to DB names
Converter->>Converter: compute constraint names and actions
Converter->>Store: add ForeignKeyConstraint / Constraint to constraints collection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@kobayashiyabako16g is attempting to deploy a commit to the Liam Team on Vercel. A member of the Team first needs to authorize it. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support to the Drizzle schema parsers (PostgreSQL + MySQL) for Drizzle v1 table-level constraints written using the (table) => [...] array-return syntax, so ERDs include the missing relationships/constraints.
Changes:
- Extend Postgres/MySQL table parsers to handle
ArrayExpressionreturns for table-level definitions (FKs, indexes, composite PKs, plus check/unique constraints). - Add AST helpers for parsing column/table references from array expressions.
- Extend converters and table definition types to carry table-level foreign keys (and, for Postgres, table-level check/unique constraints) into internal
Constraintobjects. - Add unified tests covering the new v1 array syntax parsing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/packages/schema/src/parser/drizzle/postgres/types.ts | Adds table-level FK + constraints fields to Postgres table definition types. |
| frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts | Implements array-syntax parsing for FKs/indexes/PKs/check/unique on Postgres. |
| frontend/packages/schema/src/parser/drizzle/postgres/converter.ts | Converts table-level foreign keys + constraints into internal constraint objects. |
| frontend/packages/schema/src/parser/drizzle/postgres/astUtils.ts | Adds new shared AST utilities used by the Postgres parser. |
| frontend/packages/schema/src/parser/drizzle/mysql/types.ts | Adds table-level FK + named composite PK support to MySQL table definition types. |
| frontend/packages/schema/src/parser/drizzle/mysql/tableParser.ts | Implements array-syntax parsing for FKs/indexes/PKs/check/unique on MySQL. |
| frontend/packages/schema/src/parser/drizzle/mysql/converter.ts | Converts table-level foreign keys into internal constraint objects. |
| frontend/packages/schema/src/parser/drizzle/mysql/astUtils.ts | Adds new shared AST utilities used by the MySQL parser. |
| frontend/packages/schema/src/parser/drizzle/tests/unified.test.ts | Adds test coverage for Drizzle v1 array-syntax table-level constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+206
to
+210
| return ( | ||
| isIdentifier(callExpr.callee) && | ||
| callExpr.callee.value === functionName && | ||
| callExpr.arguments.length > 0 | ||
| ) |
Comment on lines
+165
to
+166
| callExpr.callee.value === functionName && | ||
| callExpr.arguments.length > 0 |
Comment on lines
+543
to
+545
| // Extract the condition from the second argument (sql template literal) | ||
| let condition = 'true' // Default condition | ||
| if (callExpr.arguments.length > 1) { |
Comment on lines
+561
to
+576
| const firstQuasi = conditionExpr.template.quasis[0] | ||
| if (firstQuasi && firstQuasi.type === 'TemplateElement') { | ||
| // SWC TemplateElement has different structure than TypeScript's | ||
| // We need to access the raw string from the SWC AST structure | ||
| // Use property access with type checking to avoid type assertions | ||
| const hasRaw = | ||
| 'raw' in firstQuasi && typeof firstQuasi.raw === 'string' | ||
| const hasCooked = | ||
| 'cooked' in firstQuasi && typeof firstQuasi.cooked === 'string' | ||
|
|
||
| if (hasRaw) { | ||
| condition = firstQuasi.raw || '' | ||
| } else if (hasCooked) { | ||
| condition = firstQuasi.cooked || '' | ||
| } | ||
| } |
Comment on lines
+266
to
+276
| } else if (returnExpr.type === 'ArrayExpression') { | ||
| for (const element of returnExpr.elements) { | ||
| if (!element) continue | ||
| const elemExpr = getArgumentExpression(element) | ||
| if (elemExpr?.type === 'CallExpression') { | ||
| const fkDef = parseForeignKeyDefinition(elemExpr) | ||
| if (fkDef) { | ||
| table.foreignKeys = table.foreignKeys ?? [] | ||
| table.foreignKeys.push(fkDef) | ||
| } | ||
| const indexDef = parseIndexDefinition(elemExpr, 'auto') |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nUpdate in Drizzle
…nt conditions in Drizzle
Previously, only the first quasi (static string part) was extracted from
sql
template literals, so column interpolations like `${table.age}` were
dropped.
Adds `reconstructSqlTemplate()` to properly join all quasis and
MemberExpression
identifiers (falling back to "?") for both MySQL and PostgreSQL
parsers.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts (1)
319-417: Consider extracting shared parsing logic between MySQL and PostgreSQL.The
parseForeignKeyDefinition,parseForeignKeyConfig,parseForeignKeyProp,isValidForeignKeyDefinition,parseCheckConstraint, andparseUniqueConstraintfunctions are nearly identical between the MySQL and PostgreSQL parsers. Consider extracting these to a sharedcommon/orshared/module to reduce duplication and ensure consistency.This is optional given the current structure works correctly.
Also applies to: 549-689
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts` around lines 319 - 417, The MySQL and PostgreSQL parsers duplicate parsing functions (parseForeignKeyDefinition, parseForeignKeyConfig, parseForeignKeyProp, isValidForeignKeyDefinition, parseCheckConstraint, parseUniqueConstraint); extract these into a shared module (e.g., shared/parserConstraints.ts), move the implementations there, export the functions and any shared types (e.g., DrizzleForeignKeyDefinition), then update both frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts and the MySQL parser to import and use the shared functions (ensure function signatures and any helper utilities like extractTableAndColumns, extractColumnNames, isStringLiteral are either moved too or remain accessible via imports), run type checks and adjust any names so callers (parseForeignKeyDefinition, parseForeignKeyConfig, parseForeignKeyProp, isValidForeignKeyDefinition, parseCheckConstraint, parseUniqueConstraint) keep the same API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts`:
- Around line 319-417: The MySQL and PostgreSQL parsers duplicate parsing
functions (parseForeignKeyDefinition, parseForeignKeyConfig,
parseForeignKeyProp, isValidForeignKeyDefinition, parseCheckConstraint,
parseUniqueConstraint); extract these into a shared module (e.g.,
shared/parserConstraints.ts), move the implementations there, export the
functions and any shared types (e.g., DrizzleForeignKeyDefinition), then update
both frontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts and the
MySQL parser to import and use the shared functions (ensure function signatures
and any helper utilities like extractTableAndColumns, extractColumnNames,
isStringLiteral are either moved too or remain accessible via imports), run type
checks and adjust any names so callers (parseForeignKeyDefinition,
parseForeignKeyConfig, parseForeignKeyProp, isValidForeignKeyDefinition,
parseCheckConstraint, parseUniqueConstraint) keep the same API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37135ce8-16e3-4de5-8f11-d8ac521a6934
📒 Files selected for processing (5)
frontend/packages/schema/src/parser/drizzle/__tests__/unified.test.tsfrontend/packages/schema/src/parser/drizzle/mysql/astUtils.tsfrontend/packages/schema/src/parser/drizzle/mysql/tableParser.tsfrontend/packages/schema/src/parser/drizzle/postgres/astUtils.tsfrontend/packages/schema/src/parser/drizzle/postgres/tableParser.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/packages/schema/src/parser/drizzle/postgres/astUtils.ts
- frontend/packages/schema/src/parser/drizzle/mysql/astUtils.ts
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
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.
Issue
Why is this change needed?
Drizzle ORM v1 uses
(table) => [...]array syntax for table-level constraints,but this form was not parsed, causing foreign key relationships to be missing from the ERD.
Added
foreignKey()in(table) => [foreignKey({ columns, foreignColumns })]array syntax for PostgreSQLand MySQL
- Supports custom name, composite columns,
onDelete/onUpdateoptions- ref: https://orm.drizzle.team/docs/indexes-constraints#foreign-key
index()/uniqueIndex()in(table) => [index(...).on(...)]array syntax for PostgreSQL and MySQLprimaryKey()in(table) => [primaryKey({ columns })]array syntax for PostgreSQL andMySQL
check()in(table) => [check(...)]array syntax for PostgreSQLunique()in(table) => [unique(...).on(...)]array syntax for PostgreSQLChanged
tableParserto handleArrayExpressionreturn type in the third argument of table definitions(previously only
ObjectExpressionwas supported)extractColumnNames/extractTableAndColumnsAST utilities for parsing column references fromarray expressions
DrizzleForeignKeyDefinitiontype andforeignKeysfield to table definition typesDrizzleForeignKeyDefinition→ForeignKeyConstraintBackground
Although there was a comment on the issue indicating that the implementation was already in progress, I have been waiting for some time and decided to submit this PR to move things forward.
If a PR from the original commenter is submitted during the review, I am happy to withdraw this one.
Summary by CodeRabbit
New Features
Tests