fix(postgres): use explicit schema in table identifier when altering …

…comments (#4123) Closes #4108 --------- Co-authored-by: Abe Neuwirth <abe@trialspark.com>
mikro-orm · Mar 12, 2023 · 60d96de · 60d96de
1 parent 22b7146
commit 60d96de
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 56 deletions.
diff --git a/packages/knex/src/schema/SchemaGenerator.ts b/packages/knex/src/schema/SchemaGenerator.ts
@@ -380,15 +380,15 @@ export class SchemaGenerator extends AbstractSchemaGenerator<AbstractSqlDriver>
       }
 
       for (const { column } of Object.values(diff.changedColumns).filter(diff => diff.changedProperties.has('autoincrement'))) {
-        this.helper.pushTableQuery(table, this.helper.getAlterColumnAutoincrement(tableName, column));
+        this.helper.pushTableQuery(table, this.helper.getAlterColumnAutoincrement(tableName, column, schemaName));
       }
 
       for (const { column, changedProperties } of Object.values(diff.changedColumns).filter(diff => diff.changedProperties.has('comment'))) {
         if (['type', 'nullable', 'autoincrement', 'unsigned', 'default', 'enumItems'].some(t => changedProperties.has(t))) {
           continue; // will be handled via knex
         }
 
-        this.helper.pushTableQuery(table, this.helper.getChangeColumnCommentSQL(tableName, column));
+        this.helper.pushTableQuery(table, this.helper.getChangeColumnCommentSQL(tableName, column, schemaName));
       }
 
       for (const [oldColumnName, column] of Object.entries(diff.renamedColumns)) {

diff --git a/packages/knex/src/schema/SchemaHelper.ts b/packages/knex/src/schema/SchemaHelper.ts
@@ -173,11 +173,11 @@ export abstract class SchemaHelper {
     return '';
   }
 
-  getAlterColumnAutoincrement(tableName: string, column: Column): string {
+  getAlterColumnAutoincrement(tableName: string, column: Column, schemaName?: string): string {
     return '';
   }
 
-  getChangeColumnCommentSQL(tableName: string, to: Column): string {
+  getChangeColumnCommentSQL(tableName: string, to: Column, schemaName?: string): string {
     return '';
   }
 

diff --git a/packages/mariadb/src/MariaDbSchemaHelper.ts b/packages/mariadb/src/MariaDbSchemaHelper.ts
@@ -240,7 +240,7 @@ export class MariaDbSchemaHelper extends SchemaHelper {
     return `alter table ${tableName} rename index ${oldIndexName} to ${keyName}`;
   }
 
-  getChangeColumnCommentSQL(tableName: string, to: Column): string {
+  getChangeColumnCommentSQL(tableName: string, to: Column, schemaName?: string): string {
     tableName = this.platform.quoteIdentifier(tableName);
     const columnName = this.platform.quoteIdentifier(to.name);
 

diff --git a/packages/mysql/src/MySqlSchemaHelper.ts b/packages/mysql/src/MySqlSchemaHelper.ts
@@ -221,7 +221,7 @@ export class MySqlSchemaHelper extends SchemaHelper {
     return `alter table ${tableName} rename index ${oldIndexName} to ${keyName}`;
   }
 
-  getChangeColumnCommentSQL(tableName: string, to: Column): string {
+  getChangeColumnCommentSQL(tableName: string, to: Column, schemaName?: string): string {
     tableName = this.platform.quoteIdentifier(tableName);
     const columnName = this.platform.quoteIdentifier(to.name);
 

diff --git a/packages/postgresql/src/PostgreSqlSchemaHelper.ts b/packages/postgresql/src/PostgreSqlSchemaHelper.ts
@@ -292,26 +292,28 @@ export class PostgreSqlSchemaHelper extends SchemaHelper {
     return ret.join(';\n');
   }
 
-  getAlterColumnAutoincrement(tableName: string, column: Column): string {
+  getAlterColumnAutoincrement(tableName: string, column: Column, schemaName?: string): string {
     const ret: string[] = [];
     const quoted = (val: string) => this.platform.quoteIdentifier(val);
+    const name = (schemaName && schemaName !== this.platform.getDefaultSchemaName() ? schemaName + '.' : '') + tableName;
 
     /* istanbul ignore else */
     if (column.autoincrement) {
       const seqName = this.platform.getIndexName(tableName, [column.name], 'sequence');
       ret.push(`create sequence if not exists ${quoted(seqName)}`);
-      ret.push(`select setval('${seqName}', (select max(${quoted(column.name)}) from ${quoted(tableName)}))`);
-      ret.push(`alter table ${quoted(tableName)} alter column ${quoted(column.name)} set default nextval('${seqName}')`);
+      ret.push(`select setval('${seqName}', (select max(${quoted(column.name)}) from ${quoted(name)}))`);
+      ret.push(`alter table ${quoted(name)} alter column ${quoted(column.name)} set default nextval('${seqName}')`);
     } else if (column.default == null) {
-      ret.push(`alter table ${quoted(tableName)} alter column ${quoted(column.name)} drop default`);
+      ret.push(`alter table ${quoted(name)} alter column ${quoted(column.name)} drop default`);
     }
 
     return ret.join(';\n');
   }
 
-  getChangeColumnCommentSQL(tableName: string, to: Column): string {
+  getChangeColumnCommentSQL(tableName: string, to: Column, schemaName?: string): string {
+    const name = this.platform.quoteIdentifier((schemaName && schemaName !== this.platform.getDefaultSchemaName() ? schemaName + '.' : '') + tableName);
     const value = to.comment ? this.platform.quoteValue(to.comment) : 'null';
-    return `comment on column "${tableName}"."${to.name}" is ${value}`;
+    return `comment on column ${name}."${to.name}" is ${value}`;
   }
 
   normalizeDefaultValue(defaultValue: string, length: number) {

diff --git a/tests/features/schema-generator/__snapshots__/comment-diffing.postgres.test.ts.snap b/tests/features/schema-generator/__snapshots__/comment-diffing.postgres.test.ts.snap
@@ -1,24 +1,24 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`comment diffing in postgres schema orm.schema updates comments 1`] = `
-"comment on column "book"."id" is 'this is primary''s key';
-comment on column "book"."name" is 'this is name of book';
-comment on table "book" is 'this is book''s table';
+exports[`comment diffing in postgres 1`] = `
+"comment on column "foo"."book"."id" is 'this is primary''s key';
+comment on column "foo"."book"."name" is 'this is name of book';
+comment on table "foo"."book" is 'this is book''s table';
 
 "
 `;
 
-exports[`comment diffing in postgres schema orm.schema updates comments 2`] = `
-"comment on column "book"."id" is 'new comment';
-comment on column "book"."name" is null;
-comment on table "book" is 'table comment';
+exports[`comment diffing in postgres 2`] = `
+"comment on column "foo"."book"."id" is 'new comment';
+comment on column "foo"."book"."name" is null;
+comment on table "foo"."book" is 'table comment';
 
 "
 `;
 
-exports[`comment diffing in postgres schema orm.schema updates comments 3`] = `
-"comment on column "book"."id" is null;
-comment on table "book" is '';
+exports[`comment diffing in postgres 3`] = `
+"comment on column "foo"."book"."id" is null;
+comment on table "foo"."book" is '';
 
 "
 `;
diff --git a/tests/features/schema-generator/comment-diffing.postgres.test.ts b/tests/features/schema-generator/comment-diffing.postgres.test.ts
@@ -45,43 +45,38 @@ export class Book3 {
 
 }
 
-describe('comment diffing in postgres', () => {
-
-  let orm: MikroORM<PostgreSqlDriver>;
-
-  beforeAll(async () => {
-    orm = await MikroORM.init({
-      entities: [Book0],
-      dbName: `mikro_orm_test_comments`,
-      driver: PostgreSqlDriver,
-    });
-    await orm.schema.ensureDatabase();
-    await orm.schema.execute('drop table if exists book');
-    await orm.schema.createSchema();
+let orm: MikroORM<PostgreSqlDriver>;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    entities: [Book0],
+    schema: 'foo',
+    dbName: `mikro_orm_test_comments`,
+    driver: PostgreSqlDriver,
   });
+  await orm.schema.refreshDatabase();
+});
 
-  afterAll(() => orm.close(true));
-
-  test('schema orm.schema updates comments', async () => {
-    await orm.discoverEntity(Book1);
-    orm.getMetadata().reset('Book0');
-    const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
-    expect(diff1).toMatchSnapshot();
-    await orm.schema.execute(diff1);
+afterAll(() => orm.close(true));
 
-    orm.getMetadata().reset('Book1');
-    await orm.discoverEntity(Book2);
-    const diff2 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
-    expect(diff2).toMatchSnapshot();
-    await orm.schema.execute(diff2);
+test('comment diffing in postgres', async () => {
+  await orm.discoverEntity(Book1);
+  orm.getMetadata().reset('Book0');
+  const diff1 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
+  expect(diff1).toMatchSnapshot();
+  await orm.schema.execute(diff1);
 
-    orm.getMetadata().reset('Book2');
-    await orm.discoverEntity(Book3);
-    const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
-    expect(diff3).toMatchSnapshot();
-    await orm.schema.execute(diff3);
+  orm.getMetadata().reset('Book1');
+  await orm.discoverEntity(Book2);
+  const diff2 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
+  expect(diff2).toMatchSnapshot();
+  await orm.schema.execute(diff2);
 
-    await expect(orm.schema.getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
-  });
+  orm.getMetadata().reset('Book2');
+  await orm.discoverEntity(Book3);
+  const diff3 = await orm.schema.getUpdateSchemaSQL({ wrap: false });
+  expect(diff3).toMatchSnapshot();
+  await orm.schema.execute(diff3);
 
+  await expect(orm.schema.getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
 });