fix(schema): rework dropping columns to support custom schemas and me…

…rge drop column queries
mikro-orm · Apr 3, 2024 · 255f425 · 255f425
1 parent ba191a2
commit 255f425
Show file tree

Hide file tree

Showing 13 changed files with 72 additions and 73 deletions.
diff --git a/packages/better-sqlite/src/BetterSqliteSchemaHelper.ts b/packages/better-sqlite/src/BetterSqliteSchemaHelper.ts
@@ -1,5 +1,5 @@
 import type { Connection, Dictionary } from '@mikro-orm/core';
-import { SchemaHelper, type AbstractSqlConnection, type IndexDef, type CheckDef } from '@mikro-orm/knex';
+import { SchemaHelper, type AbstractSqlConnection, type IndexDef, type CheckDef, type Column } from '@mikro-orm/knex';
 
 export class BetterSqliteSchemaHelper extends SchemaHelper {
 
@@ -20,6 +20,14 @@ export class BetterSqliteSchemaHelper extends SchemaHelper {
       + `union all select name as table_name from sqlite_temp_master where type = 'table' order by name`;
   }
 
+  override getDropColumnsSQL(tableName: string, columns: Column[], schemaName?: string): string {
+    const name = this.platform.quoteIdentifier((schemaName && schemaName !== this.platform.getDefaultSchemaName() ? schemaName + '.' : '') + tableName);
+
+    return columns.map(column => {
+      return `alter table ${name} drop column ${this.platform.quoteIdentifier(column.name)}`;
+    }).join(';\n');
+  }
+
   private parseTableDefinition(sql: string, cols: any[]) {
     const columns: Dictionary<{ name: string; definition: string }> = {};
 

diff --git a/packages/knex/src/schema/SchemaHelper.ts b/packages/knex/src/schema/SchemaHelper.ts
@@ -129,6 +129,13 @@ export abstract class SchemaHelper {
     return [this.getDropIndexSQL(tableName, { ...index, keyName: oldIndexName }), this.getCreateIndexSQL(tableName, index)].join(';\n');
   }
 
+  getDropColumnsSQL(tableName: string, columns: Column[], schemaName?: string): string {
+    const name = this.platform.quoteIdentifier((schemaName && schemaName !== this.platform.getDefaultSchemaName() ? schemaName + '.' : '') + tableName);
+    const drops = columns.map(column => `drop column ${this.platform.quoteIdentifier(column.name)}`).join(', ');
+
+    return `alter table ${name} ${drops}`;
+  }
+
   hasNonDefaultPrimaryKeyName(table: DatabaseTable): boolean {
     const pkIndex = table.getPrimaryKey();
 

diff --git a/packages/knex/src/schema/SqlSchemaGenerator.ts b/packages/knex/src/schema/SqlSchemaGenerator.ts
@@ -441,10 +441,8 @@ export class SqlSchemaGenerator extends AbstractSchemaGenerator<AbstractSqlDrive
       }
 
       /* istanbul ignore else */
-      if (!safe) {
-        for (const column of Object.values(diff.removedColumns)) {
-          this.helper.pushTableQuery(table, `alter table ${this.platform.quoteIdentifier(tableName)} drop column ${this.platform.quoteIdentifier(column.name)}`);
-        }
+      if (!safe && Object.values(diff.removedColumns).length > 0) {
+        this.helper.pushTableQuery(table, this.helper.getDropColumnsSQL(tableName, Object.values(diff.removedColumns), schemaName));
       }
     }));
 

diff --git a/packages/sqlite/src/SqliteSchemaHelper.ts b/packages/sqlite/src/SqliteSchemaHelper.ts
@@ -1,5 +1,5 @@
 import type { Connection, Dictionary } from '@mikro-orm/core';
-import { SchemaHelper, type AbstractSqlConnection, type IndexDef, type CheckDef } from '@mikro-orm/knex';
+import { SchemaHelper, type AbstractSqlConnection, type IndexDef, type CheckDef, type Column } from '@mikro-orm/knex';
 
 export class SqliteSchemaHelper extends SchemaHelper {
 
@@ -20,6 +20,14 @@ export class SqliteSchemaHelper extends SchemaHelper {
       + `union all select name as table_name from sqlite_temp_master where type = 'table' order by name`;
   }
 
+  override getDropColumnsSQL(tableName: string, columns: Column[], schemaName?: string): string {
+    const name = this.platform.quoteIdentifier((schemaName && schemaName !== this.platform.getDefaultSchemaName() ? schemaName + '.' : '') + tableName);
+
+    return columns.map(column => {
+      return `alter table ${name} drop column ${this.platform.quoteIdentifier(column.name)}`;
+    }).join(';\n');
+  }
+
   private parseTableDefinition(sql: string, cols: any[]) {
     const columns: Dictionary<{ name: string; definition: string }> = {};
 

diff --git a/tests/features/migrations/__snapshots__/Migrator.postgres.test.ts.snap b/tests/features/migrations/__snapshots__/Migrator.postgres.test.ts.snap
@@ -385,11 +385,11 @@ const { Migration } = require('@mikro-orm/migrations');
 class Migration20191013214813 extends Migration {
 
   async up() {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down() {
@@ -410,11 +410,11 @@ exports.Migration20191013214813 = Migration20191013214813;
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "Migration20191013214813.js",
@@ -430,11 +430,11 @@ import { Migration } from '@mikro-orm/migrations';
 export class Migration20191013214813 extends Migration {
 
   async up(): Promise<void> {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down(): Promise<void> {
@@ -454,11 +454,11 @@ export class Migration20191013214813 extends Migration {
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "Migration20191013214813.ts",
@@ -472,11 +472,11 @@ exports[`Migrator (postgres) generate migration with custom name with name optio
 export class Migration20191013214813_custom_name extends Migration {
 
   async up(): Promise<void> {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down(): Promise<void> {
@@ -496,11 +496,11 @@ export class Migration20191013214813_custom_name extends Migration {
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "migration20191013214813_custom_name.ts",
@@ -514,11 +514,11 @@ exports[`Migrator (postgres) generate migration with custom name: migration-dump
 export class Migration20191013214813 extends Migration {
 
   async up(): Promise<void> {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down(): Promise<void> {
@@ -538,11 +538,11 @@ export class Migration20191013214813 extends Migration {
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "migration-20191013214813.ts",
@@ -556,11 +556,11 @@ exports[`Migrator (postgres) generate migration with snapshot: migration-snapsho
 export class Migration20191013214813 extends Migration {
 
   async up(): Promise<void> {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down(): Promise<void> {
@@ -580,11 +580,11 @@ export class Migration20191013214813 extends Migration {
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "Migration20191013214813.ts",
@@ -609,11 +609,11 @@ exports[`Migrator (postgres) generate schema migration: migration-dump 1`] = `
 export class Migration20191013214813 extends Migration {
 
   async up(): Promise<void> {
-    this.addSql('alter table "book2" drop column "foo";');
+    this.addSql('alter table "custom"."book2" drop column "foo";');
 
     this.addSql('alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);');
 
-    this.addSql('alter table "test2" drop column "path";');
+    this.addSql('alter table "custom"."test2" drop column "path";');
   }
 
   async down(): Promise<void> {
@@ -633,11 +633,11 @@ export class Migration20191013214813 extends Migration {
       "alter table "custom"."test2" add column "path" polygon null;",
     ],
     "up": [
-      "alter table "book2" drop column "foo";",
+      "alter table "custom"."book2" drop column "foo";",
       "",
       "alter table "custom"."book2" alter column "double" type double precision using ("double"::double precision);",
       "",
-      "alter table "test2" drop column "path";",
+      "alter table "custom"."test2" drop column "path";",
     ],
   },
   "fileName": "Migration20191013214813.ts",

diff --git a/tests/features/migrations/__snapshots__/Migrator.test.ts.snap b/tests/features/migrations/__snapshots__/Migrator.test.ts.snap
@@ -344,8 +344,7 @@ class Migration20191013214813 extends Migration {
     this.addSql('alter table \`book2\` drop column \`foo\`;');
 
     this.addSql('alter table \`test2\` drop index \`test2_foo___bar_unique\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___bar\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___baz\`;');
+    this.addSql('alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;');
   }
 
   async down() {
@@ -373,8 +372,7 @@ exports.Migration20191013214813 = Migration20191013214813;
       "alter table \`book2\` drop column \`foo\`;",
       "",
       "alter table \`test2\` drop index \`test2_foo___bar_unique\`;",
-      "alter table \`test2\` drop column \`foo___bar\`;",
-      "alter table \`test2\` drop column \`foo___baz\`;",
+      "alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;",
     ],
   },
   "fileName": "Migration20191013214813.cjs",
@@ -395,8 +393,7 @@ class Migration20191013214813 extends Migration {
     this.addSql('alter table \`book2\` drop column \`foo\`;');
 
     this.addSql('alter table \`test2\` drop index \`test2_foo___bar_unique\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___bar\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___baz\`;');
+    this.addSql('alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;');
   }
 
   async down() {
@@ -424,8 +421,7 @@ exports.Migration20191013214813 = Migration20191013214813;
       "alter table \`book2\` drop column \`foo\`;",
       "",
       "alter table \`test2\` drop index \`test2_foo___bar_unique\`;",
-      "alter table \`test2\` drop column \`foo___bar\`;",
-      "alter table \`test2\` drop column \`foo___baz\`;",
+      "alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;",
     ],
   },
   "fileName": "Migration20191013214813.js",
@@ -444,8 +440,7 @@ export class Migration20191013214813 extends Migration {
     this.addSql('alter table \`book2\` drop column \`foo\`;');
 
     this.addSql('alter table \`test2\` drop index \`test2_foo___bar_unique\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___bar\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___baz\`;');
+    this.addSql('alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;');
   }
 
   async down(): Promise<void> {
@@ -472,8 +467,7 @@ export class Migration20191013214813 extends Migration {
       "alter table \`book2\` drop column \`foo\`;",
       "",
       "alter table \`test2\` drop index \`test2_foo___bar_unique\`;",
-      "alter table \`test2\` drop column \`foo___bar\`;",
-      "alter table \`test2\` drop column \`foo___baz\`;",
+      "alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;",
     ],
   },
   "fileName": "migration-20191013214813.ts",
@@ -492,8 +486,7 @@ export class Migration20191013214813 extends Migration {
     this.addSql('alter table \`book2\` drop column \`foo\`;');
 
     this.addSql('alter table \`test2\` drop index \`test2_foo___bar_unique\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___bar\`;');
-    this.addSql('alter table \`test2\` drop column \`foo___baz\`;');
+    this.addSql('alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;');
   }
 
   async down(): Promise<void> {
@@ -520,8 +513,7 @@ export class Migration20191013214813 extends Migration {
       "alter table \`book2\` drop column \`foo\`;",
       "",
       "alter table \`test2\` drop index \`test2_foo___bar_unique\`;",
-      "alter table \`test2\` drop column \`foo___bar\`;",
-      "alter table \`test2\` drop column \`foo___baz\`;",
+      "alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;",
     ],
   },
   "fileName": "Migration20191013214813.ts",

diff --git a/tests/features/schema-generator/__snapshots__/GH4919.test.ts.snap b/tests/features/schema-generator/__snapshots__/GH4919.test.ts.snap
@@ -13,8 +13,7 @@ exports[`GH #4919: 0. create schema 1`] = `
 
 exports[`GH #4919: 1. rename column 1`] = `
 {
-  "down": "alter table "user" drop column "canceled_at";
-alter table "user" drop column "arrived_at";
+  "down": "alter table "user" drop column "canceled_at", drop column "arrived_at";
 
 alter table "user" rename column "completed_at" to "delivered_at";
 

diff --git a/tests/features/schema-generator/__snapshots__/SchemaGenerator.mariadb.test.ts.snap b/tests/features/schema-generator/__snapshots__/SchemaGenerator.mariadb.test.ts.snap
@@ -192,8 +192,7 @@ alter table \`test2\` drop foreign key \`test2_foo___bar_foreign\`;
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 set foreign_key_checks = 1;
 "

diff --git a/tests/features/schema-generator/__snapshots__/SchemaGenerator.mysql.test.ts.snap b/tests/features/schema-generator/__snapshots__/SchemaGenerator.mysql.test.ts.snap
@@ -192,8 +192,7 @@ alter table \`test2\` drop foreign key \`test2_foo___bar_foreign\`;
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 set foreign_key_checks = 1;
 "
@@ -216,8 +215,7 @@ alter table \`author2\` add index \`author2_name_age_in_years_index\`(\`name\`,
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 "
 `;
@@ -263,8 +261,7 @@ alter table \`test2\` drop foreign key \`test2_foo___bar_foreign\`;
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 "
 `;
@@ -284,8 +281,7 @@ exports[`SchemaGenerator update schema [mysql]: mysql-update-schema-drop-column
 alter table \`author2\` drop foreign key \`author2_favourite_book_uuid_pk_foreign\`;
 
 alter table \`new_table\` drop primary key;
-alter table \`new_table\` drop column \`id\`;
-alter table \`new_table\` drop column \`updated_at\`;
+alter table \`new_table\` drop column \`id\`, drop column \`updated_at\`;
 
 alter table \`author2\` drop index \`author2_favourite_book_uuid_pk_index\`;
 alter table \`author2\` drop column \`favourite_book_uuid_pk\`;
@@ -351,8 +347,7 @@ alter table \`test2\` drop foreign key \`test2_foo___bar_foreign\`;
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 "
 `;

diff --git a/tests/features/schema-generator/__snapshots__/SchemaGenerator.mysql2.test.ts.snap b/tests/features/schema-generator/__snapshots__/SchemaGenerator.mysql2.test.ts.snap
@@ -164,8 +164,7 @@ exports[`SchemaGenerator (no FKs) generate schema from metadata [mysql]: mysql-u
 alter table \`book2\` drop column \`foo\`;
 
 alter table \`test2\` drop index \`test2_foo___bar_unique\`;
-alter table \`test2\` drop column \`foo___bar\`;
-alter table \`test2\` drop column \`foo___baz\`;
+alter table \`test2\` drop column \`foo___bar\`, drop column \`foo___baz\`;
 
 "
 `;