fix(schema): respect up migration when detecting column renaming in d…

…own migration

Closes #4919

packages/knex/src/schema/SchemaComparator.ts

@@ -31,7 +31,7 @@ export class SchemaComparator {
    * operations to change the schema stored in fromSchema to the schema that is
    * stored in toSchema.
    */
-  compare(fromSchema: DatabaseSchema, toSchema: DatabaseSchema): SchemaDifference {
+  compare(fromSchema: DatabaseSchema, toSchema: DatabaseSchema, inverseDiff?: SchemaDifference): SchemaDifference {
     const diff: SchemaDifference = { newTables: {}, removedTables: {}, changedTables: {}, orphanedForeignKeys: [], newNamespaces: new Set(), removedNamespaces: new Set(), fromSchema };
     const foreignKeysToTable: Dictionary<ForeignKey[]> = {};
 
@@ -57,7 +57,7 @@ export class SchemaComparator {
       if (!fromSchema.hasTable(tableName)) {
         diff.newTables[tableName] = toSchema.getTable(tableName)!;
       } else {
-        const tableDifferences = this.diffTable(fromSchema.getTable(tableName)!, toSchema.getTable(tableName)!);
+        const tableDifferences = this.diffTable(fromSchema.getTable(tableName)!, toSchema.getTable(tableName)!, inverseDiff?.changedTables[tableName]);
 
         if (tableDifferences !== false) {
           diff.changedTables[tableName] = tableDifferences;
@@ -119,7 +119,7 @@ export class SchemaComparator {
    * Returns the difference between the tables fromTable and toTable.
    * If there are no differences this method returns the boolean false.
    */
-  diffTable(fromTable: DatabaseTable, toTable: DatabaseTable): TableDifference | false {
+  diffTable(fromTable: DatabaseTable, toTable: DatabaseTable, inverseTableDiff?: TableDifference): TableDifference | false {
     let changes = 0;
     const tableDifferences: TableDifference = {
       name: fromTable.getShortestName(),
@@ -188,7 +188,7 @@ export class SchemaComparator {
       changes++;
     }
 
-    this.detectColumnRenamings(tableDifferences);
+    this.detectColumnRenamings(tableDifferences, inverseTableDiff);
     const fromTableIndexes = fromTable.getIndexes();
     const toTableIndexes = toTable.getIndexes();
 
@@ -299,16 +299,23 @@ export class SchemaComparator {
    * Try to find columns that only changed their name, rename operations maybe cheaper than add/drop
    * however ambiguities between different possibilities should not lead to renaming at all.
    */
-  private detectColumnRenamings(tableDifferences: TableDifference): void {
+  private detectColumnRenamings(tableDifferences: TableDifference, inverseTableDiff?: TableDifference): void {
     const renameCandidates: Dictionary<[Column, Column][]> = {};
 
     for (const addedColumn of Object.values(tableDifferences.addedColumns)) {
       for (const removedColumn of Object.values(tableDifferences.removedColumns)) {
         const diff = this.diffColumn(addedColumn, removedColumn);
+
         if (diff.size !== 0) {
           continue;
         }
 
+        const renamedColumn = inverseTableDiff?.renamedColumns[addedColumn.name];
+
+        if (renamedColumn && renamedColumn?.name !== removedColumn.name) {
+          continue;
+        }
+
         renameCandidates[addedColumn.name] = renameCandidates[addedColumn.name] ?? [];
         renameCandidates[addedColumn.name].push([removedColumn, addedColumn]);
       }

packages/knex/src/schema/SchemaGenerator.ts

@@ -186,7 +186,7 @@ export class SchemaGenerator extends AbstractSchemaGenerator<AbstractSqlDriver>
     const { fromSchema, toSchema } = await this.prepareSchemaForComparison(options);
     const comparator = new SchemaComparator(this.platform);
     const diffUp = comparator.compare(fromSchema, toSchema);
-    const diffDown = comparator.compare(toSchema, fromSchema);
+    const diffDown = comparator.compare(toSchema, fromSchema, diffUp);
 
     return {
       up: await this.diffToSQL(diffUp, options),

tests/features/schema-generator/GH4919.test.ts

@@ -0,0 +1,66 @@
+import { MikroORM } from '@mikro-orm/postgresql';
+import { Entity, PrimaryKey, Property } from '@mikro-orm/core';
+
+@Entity({ tableName: 'user' })
+class User0 {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  deliveredAt!: Date;
+
+}
+
+@Entity({ tableName: 'user' })
+class User1 {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  completedAt!: Date;
+
+  @Property()
+  canceledAt!: Date;
+
+  @Property()
+  arrivedAt!: Date;
+
+}
+
+let orm: MikroORM;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    entities: [User0],
+    dbName: 'mikro_orm_test_gh_4919',
+  });
+  await orm.schema.ensureDatabase();
+  await orm.schema.dropSchema();
+});
+
+afterAll(() => orm.close(true));
+
+test('4782', async () => {
+  const testMigration = async (e1: any, e2: any, snap: string) => {
+    if (e2) {
+      orm.getMetadata().reset(e1.name);
+      await orm.discoverEntity(e2);
+    }
+
+    const diff = await orm.schema.getUpdateSchemaMigrationSQL({ wrap: false });
+    expect(diff).toMatchSnapshot(snap);
+    await orm.schema.execute(diff.up);
+
+    return diff.down;
+  };
+
+  const down: string[] = [];
+  down.push(await testMigration(User0, undefined, '0. create schema'));
+  down.push(await testMigration(User0, User1, '1. rename column'));
+
+  for (const sql of down.reverse()) {
+    await orm.schema.execute(sql);
+  }
+});

tests/features/schema-generator/__snapshots__/GH4919.test.ts.snap

@@ -0,0 +1,27 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`4782: 0. create schema 1`] = `
+{
+  "down": "drop table if exists "user" cascade;
+
+",
+  "up": "create table "user" ("id" serial primary key, "delivered_at" timestamptz not null);
+
+",
+}
+`;
+
+exports[`4782: 1. rename column 1`] = `
+{
+  "down": "alter table "user" drop column "canceled_at";
+alter table "user" drop column "arrived_at";
+
+alter table "user" rename column "completed_at" to "delivered_at";
+
+",
+  "up": "alter table "user" add column "canceled_at" timestamptz not null, add column "arrived_at" timestamptz not null;
+alter table "user" rename column "delivered_at" to "completed_at";
+
+",
+}
+`;