Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions packages/cli/src/actions/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,23 +168,29 @@ async function runPull(options: PullOptions) {
});
resolvedRelations.push(...relations);
}
// Normalize schema values: MySQL uses null for references.schema but '' for table.schema;
// treat null, undefined, and '' as equivalent when comparing schemas.
const schemasMatch = (a: string | null | undefined, b: string | null | undefined) =>
(a ?? '') === (b ?? '');

// sync relation fields
for (const relation of resolvedRelations) {
const similarRelations = resolvedRelations.filter((rr) => {
return (
rr !== relation &&
((rr.schema === relation.schema &&
((schemasMatch(rr.schema, relation.schema) &&
rr.table === relation.table &&
rr.references.schema === relation.references.schema &&
schemasMatch(rr.references.schema, relation.references.schema) &&
rr.references.table === relation.references.table) ||
(rr.schema === relation.references.schema &&
rr.columns[0] === relation.references.columns[0] &&
rr.references.schema === relation.schema &&
(schemasMatch(rr.schema, relation.references.schema) &&
rr.table === relation.references.table &&
schemasMatch(rr.references.schema, relation.schema) &&
rr.references.table === relation.table))
);
}).length;
Comment on lines 178 to 190

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve relation-column identity when counting similar relations.

This now treats every relation between the same table pair as similar, even when they use different foreign-key columns (for example, createdById versus updatedById). That can make unrelated relations share the same similarity count and cause incorrect relation-name synchronization. Keep a direction-aware comparison of the local and referenced column arrays in addition to the schema/table checks.

Suggested direction-aware check
+const columnsMatch = (a: string[], b: string[]) =>
+    a.length === b.length && a.every((column, index) => column === b[index]);
+
 const similarRelations = resolvedRelations.filter((rr) => {
     return (
         rr !== relation &&
         ((schemasMatch(rr.schema, relation.schema) &&
             rr.table === relation.table &&
+            columnsMatch(rr.columns, relation.columns) &&
             schemasMatch(rr.references.schema, relation.references.schema) &&
-            rr.references.table === relation.references.table) ||
+            rr.references.table === relation.references.table &&
+            columnsMatch(rr.references.columns, relation.references.columns)) ||
             (schemasMatch(rr.schema, relation.references.schema) &&
                 rr.table === relation.references.table &&
+                columnsMatch(rr.columns, relation.references.columns) &&
                 schemasMatch(rr.references.schema, relation.schema) &&
-                rr.references.table === relation.table))
+                rr.references.table === relation.table &&
+                columnsMatch(rr.references.columns, relation.columns)))
     );
});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const similarRelations = resolvedRelations.filter((rr) => {
return (
rr !== relation &&
((rr.schema === relation.schema &&
((schemasMatch(rr.schema, relation.schema) &&
rr.table === relation.table &&
rr.references.schema === relation.references.schema &&
schemasMatch(rr.references.schema, relation.references.schema) &&
rr.references.table === relation.references.table) ||
(rr.schema === relation.references.schema &&
rr.columns[0] === relation.references.columns[0] &&
rr.references.schema === relation.schema &&
(schemasMatch(rr.schema, relation.references.schema) &&
rr.table === relation.references.table &&
schemasMatch(rr.references.schema, relation.schema) &&
rr.references.table === relation.table))
);
}).length;
const columnsMatch = (a: string[], b: string[]) =>
a.length === b.length && a.every((column, index) => column === b[index]);
const similarRelations = resolvedRelations.filter((rr) => {
return (
rr !== relation &&
((schemasMatch(rr.schema, relation.schema) &&
rr.table === relation.table &&
columnsMatch(rr.columns, relation.columns) &&
schemasMatch(rr.references.schema, relation.references.schema) &&
rr.references.table === relation.references.table &&
columnsMatch(rr.references.columns, relation.references.columns)) ||
(schemasMatch(rr.schema, relation.references.schema) &&
rr.table === relation.references.table &&
columnsMatch(rr.columns, relation.references.columns) &&
schemasMatch(rr.references.schema, relation.schema) &&
rr.references.table === relation.table &&
columnsMatch(rr.references.columns, relation.columns)))
);
}).length;
🤖 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/cli/src/actions/db.ts` around lines 178 - 190, Update the
similarRelations filter in the resolvedRelations counting logic to compare
relation-column identity in addition to schema and table matches. Preserve the
existing direction-aware branches, requiring each branch to match the
corresponding local and referenced column arrays while keeping reversed
relations mapped to the opposite arrays.

const selfRelation =
relation.references.schema === relation.schema && relation.references.table === relation.table;
schemasMatch(relation.references.schema, relation.schema) &&
relation.references.table === relation.table;
syncRelation({
model: newModel,
relation,
Expand Down
91 changes: 64 additions & 27 deletions packages/cli/test/db/pull.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ model User {
const restoredSchema = getSchema(workDir);
expect(restoredSchema).toEqual(schema);
});

});

describe('Pull with existing schema - preserve schema features', () => {
Expand Down Expand Up @@ -424,10 +423,9 @@ enum Role {
USER
ADMIN
MODERATOR
}`,
// When using MySQL, the introspection simply overrides the enum and cannot detect if it exists with the same name because it only stores the values.
// TODO: Create a better way to handle this, possibly by finding enums by their values as well if the schema exists.
);
}`);
// When using MySQL, the introspection simply overrides the enum and cannot detect if it exists with the same name because it only stores the values.
// TODO: Create a better way to handle this, possibly by finding enums by their values as well if the schema exists.
runCli('db push', workDir);

runCli('db pull --indent 4', workDir);
Expand Down Expand Up @@ -486,10 +484,33 @@ model Post {
expect(pulledUserSchema).toEqual(userModel);
expect(pulledPostSchema).toEqual(postModel);
});

it('should preserve opposite-direction relations between the same two tables', async () => {
const { workDir, schema } = await createProject(
`model User {
id Int @id @default(autoincrement())
postId Int? @unique
favPost Post? @relation('FavPost', fields: [postId], references: [id])
posts Post[] @relation('UserPosts')
}

model Post {
id Int @id @default(autoincrement())
userId Int
author User @relation('UserPosts', fields: [userId], references: [id])
favUser User? @relation('FavPost')
}`,
);
runCli('db push', workDir);

runCli('db pull --indent 4', workDir);

const preservedSchema = getSchema(workDir);
expect(preservedSchema).toEqual(schema);
});
});

describe('Pull should preserve enum declaration order', () => {

it('should preserve interleaved enum and model ordering', async () => {
const { workDir, schema } = await createProject(
`enum Role {
Expand Down Expand Up @@ -824,13 +845,16 @@ model Post {

@@schema('content')
}`,
{ provider: 'postgresql', datasourceFields:{ schemas: ['public', 'content', 'auth'] } },
{ provider: 'postgresql', datasourceFields: { schemas: ['public', 'content', 'auth'] } },
);
runCli('db push', workDir);

const schemaFile = path.join(workDir, 'zenstack/schema.zmodel');

fs.writeFileSync(schemaFile, getDefaultPrelude({ provider: 'postgresql', datasourceFields:{ schemas: ['public', 'content', 'auth']} }));
fs.writeFileSync(
schemaFile,
getDefaultPrelude({ provider: 'postgresql', datasourceFields: { schemas: ['public', 'content', 'auth'] } }),
);
runCli('db pull --indent 4', workDir);

const restoredSchema = getSchema(workDir);
Expand Down Expand Up @@ -906,7 +930,7 @@ enum Status {
INACTIVE
SUSPENDED
}`,
{ provider: 'postgresql', datasourceFields:{ schemas: ['public', 'content', 'auth'] } },
{ provider: 'postgresql', datasourceFields: { schemas: ['public', 'content', 'auth'] } },
);
runCli('db push', workDir);

Expand Down Expand Up @@ -1040,7 +1064,7 @@ model Tenant {
await client.connect();
try {
await client.query(
`ALTER TABLE "ComputedUsers" ADD COLUMN "fullName" text GENERATED ALWAYS AS ("firstName" || ' ' || "lastName") STORED`
`ALTER TABLE "ComputedUsers" ADD COLUMN "fullName" text GENERATED ALWAYS AS ("firstName" || ' ' || "lastName") STORED`,
);
} finally {
await client.end();
Expand All @@ -1055,14 +1079,16 @@ model Tenant {

// The generated column should be pulled as Unsupported with the full expression.
// format_type returns 'text', and pg_get_expr returns the expression.
expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude({ provider: 'postgresql' })}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude({ provider: 'postgresql' })}

model ComputedUsers {
id Int @id @default(autoincrement())
firstName String
lastName String
fullName Unsupported('text GENERATED ALWAYS AS ((("firstName" || \\' \\'::text) || "lastName")) STORED')?
}`));
}`),
);
});

it('should pull virtual generated columns as Unsupported with full expression', async ({ skip }) => {
Expand Down Expand Up @@ -1091,7 +1117,7 @@ model ComputedUsers {
await client.connect();
try {
await client.query(
`ALTER TABLE "ComputedProducts" ADD COLUMN "total" integer GENERATED ALWAYS AS ("price" * "qty") STORED`
`ALTER TABLE "ComputedProducts" ADD COLUMN "total" integer GENERATED ALWAYS AS ("price" * "qty") STORED`,
);
} finally {
await client.end();
Expand All @@ -1103,14 +1129,16 @@ model ComputedUsers {

const restoredSchema = getSchema(workDir);

expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude({ provider: 'postgresql' })}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude({ provider: 'postgresql' })}

model ComputedProducts {
id Int @id @default(autoincrement())
price Int @default(0)
qty Int @default(0)
total Unsupported('integer GENERATED ALWAYS AS ((price * qty)) STORED')?
}`));
}`),
);
});
});

Expand Down Expand Up @@ -1173,7 +1201,7 @@ describe('DB pull - MySQL specific features', () => {
const connection = await mysql.createConnection(getTestDbUrl('mysql', dbName));
try {
await connection.execute(
"ALTER TABLE `ComputedUsers` ADD COLUMN `fullName` varchar(511) GENERATED ALWAYS AS (CONCAT(`firstName`, ' ', `lastName`)) STORED"
"ALTER TABLE `ComputedUsers` ADD COLUMN `fullName` varchar(511) GENERATED ALWAYS AS (CONCAT(`firstName`, ' ', `lastName`)) STORED",
);
} finally {
await connection.end();
Expand All @@ -1189,14 +1217,16 @@ describe('DB pull - MySQL specific features', () => {
// The generated column should be pulled as Unsupported with the full expression.
// MySQL uses COLUMN_TYPE (e.g., 'varchar(511)') and GENERATION_EXPRESSION for the expr,
// and EXTRA contains 'STORED GENERATED' or 'VIRTUAL GENERATED'.
expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude({ provider: 'mysql' })}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude({ provider: 'mysql' })}

model ComputedUsers {
id Int @id @default(autoincrement())
firstName String @db.VarChar(255)
lastName String @db.VarChar(255)
fullName Unsupported('varchar(511) GENERATED ALWAYS AS (concat(\`firstName\`,\\' \\',\`lastName\`)) STORED')?
}`));
}`),
);
});

it('should pull virtual generated columns as Unsupported with full expression', async ({ skip }) => {
Expand All @@ -1221,7 +1251,7 @@ model ComputedUsers {
const connection = await mysql.createConnection(getTestDbUrl('mysql', dbName));
try {
await connection.execute(
"ALTER TABLE `ComputedProducts` ADD COLUMN `total` int GENERATED ALWAYS AS (`price` * `qty`) VIRTUAL"
'ALTER TABLE `ComputedProducts` ADD COLUMN `total` int GENERATED ALWAYS AS (`price` * `qty`) VIRTUAL',
);
} finally {
await connection.end();
Expand All @@ -1233,14 +1263,16 @@ model ComputedUsers {

const restoredSchema = getSchema(workDir);

expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude({ provider: 'mysql' })}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude({ provider: 'mysql' })}

model ComputedProducts {
id Int @id @default(autoincrement())
price Int @default(0)
qty Int @default(0)
total Unsupported('int GENERATED ALWAYS AS ((\`price\` * \`qty\`)) VIRTUAL')?
}`));
}`),
);
});
});

Expand Down Expand Up @@ -1295,7 +1327,7 @@ model Tenant {
return;
}
// Create a minimal project and push to get the database file.
const { workDir } = await createProject("");
const { workDir } = await createProject('');

// Open the SQLite database directly and add a table with an untyped column.
// In SQLite, CREATE TABLE t("data") gives column "data" no declared type,
Expand Down Expand Up @@ -1351,14 +1383,16 @@ model Tenant {
const restoredSchema = getSchema(workDir);

// first_name and last_name should be regular String fields
expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude()}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude()}

model ComputedUsers {
id Int @id @default(autoincrement())
firstName String
lastName String
fullName Unsupported('TEXT GENERATED ALWAYS AS (firstName || \\' \\' || lastName) STORED')?
}`));
}`),
);
});

it('should pull virtual generated columns as Unsupported', async ({ skip }) => {
Expand Down Expand Up @@ -1389,14 +1423,16 @@ model ComputedUsers {

const restoredSchema = getSchema(workDir);

expect(restoredSchema).toEqual(await formatDocument(`${getDefaultPrelude()}
expect(restoredSchema).toEqual(
await formatDocument(`${getDefaultPrelude()}

model ComputedProducts {
id Int @id @default(autoincrement())
price Int @default(0)
qty Int @default(0)
total Unsupported('INTEGER GENERATED ALWAYS AS ("price" * "qty") VIRTUAL')?
}`));
}`),
);
});
});

Expand All @@ -1419,7 +1455,8 @@ enum UserStatus {
ACTIVE
INACTIVE
SUSPENDED
}`);
}`,
);
runCli('db push', workDir);

const schemaFile = path.join(workDir, 'zenstack/schema.zmodel');
Expand Down