refactor: fix diffing of schemas/namespaces

Fixes mostly postgres handling multiple schemas and diffing, also fixes diffing
when explicit public schema is provided.

Related: #2621

packages/core/src/metadata/MetadataDiscovery.ts

@@ -526,7 +526,8 @@ export class MetadataDiscovery {
     ret.targetMeta = meta;
     ret.joinColumns = [];
     ret.inverseJoinColumns = [];
-    ret.referencedTableName = meta.collection;
+    const schema = meta.schema ?? this.platform.getDefaultSchemaName();
+    ret.referencedTableName = schema ? schema + '.' + meta.tableName : meta.tableName;
 
     if (owner) {
       ret.owner = true;
@@ -546,7 +547,6 @@ export class MetadataDiscovery {
       ret.fieldNames = ret.joinColumns = prop.inverseJoinColumns;
       ret.referencedColumnNames = [];
       ret.inverseJoinColumns = [];
-      ret.referencedTableName = meta.collection;
       meta.primaryKeys.forEach(primaryKey => {
         const prop2 = meta.properties[primaryKey];
         ret.referencedColumnNames.push(...prop2.fieldNames);

packages/core/src/platforms/Platform.ts

@@ -152,6 +152,10 @@ export abstract class Platform {
     return typeof value === 'object' && value !== null && '__raw' in value;
   }
 
+  getDefaultSchemaName(): string | undefined {
+    return undefined;
+  }
+
   getBooleanTypeDeclarationSQL(): string {
     return 'boolean';
   }

packages/core/src/typings.ts

@@ -298,6 +298,14 @@ export class EntityMetadata<T extends AnyEntity<T> = any> {
     return this.primaryKeys.map(pk => this.properties[pk]);
   }
 
+  get tableName(): string {
+    return this.collection;
+  }
+
+  set tableName(name: string) {
+    this.collection = name;
+  }
+
   sync(initIndexes = false) {
     this.root ??= this;
     const props = Object.values(this.properties).sort((a, b) => this.propertyOrder.get(a.name)! - this.propertyOrder.get(b.name)!);

packages/entity-generator/src/SourceFile.ts

@@ -62,7 +62,7 @@ export class SourceFile {
       options.tableName = quote(this.meta.collection);
     }
 
-    if (this.meta.schema) {
+    if (this.meta.schema && this.meta.schema !== this.platform.getDefaultSchemaName()) {
       options.schema = quote(this.meta.schema);
     }
 

packages/knex/src/query/QueryBuilderHelper.ts

@@ -173,12 +173,13 @@ export class QueryBuilderHelper {
   }
 
   joinPivotTable(field: string, prop: EntityProperty, ownerAlias: string, alias: string, type: 'leftJoin' | 'innerJoin' | 'pivotJoin', cond: Dictionary = {}): JoinOptions {
-    const prop2 = this.metadata.find(field)!.properties[prop.mappedBy || prop.inversedBy];
+    const pivotMeta = this.metadata.find(field)!;
+    const prop2 = pivotMeta.properties[prop.mappedBy || prop.inversedBy];
 
     return {
       prop, type, cond, ownerAlias, alias,
-      table: this.metadata.find(field)!.collection,
-      schema: prop2.targetMeta!.schema,
+      table: pivotMeta.collection,
+      schema: pivotMeta.schema,
       joinColumns: prop.joinColumns,
       inverseJoinColumns: prop2.joinColumns,
       primaryKeys: prop.referencedColumnNames,

packages/knex/src/schema/DatabaseTable.ts

@@ -16,7 +16,11 @@ export class DatabaseTable {
 
   constructor(private readonly platform: AbstractSqlPlatform,
               readonly name: string,
-              readonly schema?: string) { }
+              readonly schema?: string) {
+    Object.defineProperties(this, {
+      platform: { enumerable: false, writable: true },
+    });
+  }
 
   getColumns(): Column[] {
     return Object.values(this.columns);
@@ -92,7 +96,12 @@ export class DatabaseTable {
 
     if ([ReferenceType.MANY_TO_ONE, ReferenceType.ONE_TO_ONE].includes(prop.reference)) {
       const constraintName = this.getIndexName(true, prop.fieldNames, 'foreign');
-      const schema = prop.targetMeta!.schema === '*' ? this.schema : prop.targetMeta!.schema;
+      let schema = prop.targetMeta!.schema === '*' ? this.schema : prop.targetMeta!.schema ?? this.schema;
+
+      if (prop.referencedTableName.includes('.')) {
+        schema = undefined;
+      }
+
       this.foreignKeys[constraintName] = {
         constraintName,
         columnNames: prop.fieldNames,
@@ -183,8 +192,8 @@ export class DatabaseTable {
   /**
    * The shortest name is stripped of the default namespace. All other namespaced elements are returned as full-qualified names.
    */
-  getShortestName(defaultNamespaceName?: string): string {
-    if (!this.schema || this.schema === defaultNamespaceName || this.name.startsWith(this.schema + '.')) {
+  getShortestName(): string {
+    if (!this.schema || this.name.startsWith(this.schema + '.')) {
       return this.name;
     }
 

packages/knex/src/schema/SchemaComparator.ts

@@ -26,23 +26,23 @@ export class SchemaComparator {
     const foreignKeysToTable: Dictionary<ForeignKey[]> = {};
 
     for (const namespace of toSchema.getNamespaces()) {
-      if (fromSchema.hasNamespace(namespace)) {
+      if (fromSchema.hasNamespace(namespace) || namespace === this.platform.getDefaultSchemaName()) {
         continue;
       }
 
       diff.newNamespaces.add(namespace);
     }
 
     for (const namespace of fromSchema.getNamespaces()) {
-      if (toSchema.hasNamespace(namespace)) {
+      if (toSchema.hasNamespace(namespace) || namespace === this.platform.getDefaultSchemaName()) {
         continue;
       }
 
       diff.removedNamespaces.add(namespace);
     }
 
     for (const table of toSchema.getTables()) {
-      const tableName = table.getShortestName(toSchema.name);
+      const tableName = table.getShortestName();
 
       if (!fromSchema.hasTable(tableName)) {
         diff.newTables[tableName] = toSchema.getTable(tableName)!;
@@ -57,7 +57,7 @@ export class SchemaComparator {
 
     // Check if there are tables removed
     for (let table of fromSchema.getTables()) {
-      const tableName = table.getShortestName(fromSchema.name);
+      const tableName = table.getShortestName();
       table = fromSchema.getTable(tableName)!;
 
       if (!toSchema.hasTable(tableName)) {
@@ -75,14 +75,16 @@ export class SchemaComparator {
     }
 
     for (const table of Object.values(diff.removedTables)) {
-      if (!foreignKeysToTable[table.name]) {
+      const tableName = (table.schema ? table.schema + '.' : '') + table.name;
+
+      if (!foreignKeysToTable[tableName]) {
         continue;
       }
 
-      diff.orphanedForeignKeys.push(...foreignKeysToTable[table.name]!);
+      diff.orphanedForeignKeys.push(...foreignKeysToTable[tableName]);
 
       // Deleting duplicated foreign keys present both on the orphanedForeignKey and the removedForeignKeys from changedTables.
-      for (const foreignKey of foreignKeysToTable[table.name]) {
+      for (const foreignKey of foreignKeysToTable[tableName]) {
         const localTableName = foreignKey.localTableName;
 
         if (!diff.changedTables[localTableName]) {
@@ -91,7 +93,7 @@ export class SchemaComparator {
 
         for (const [key, fk] of Object.entries(diff.changedTables[localTableName].removedForeignKeys)) {
           // We check if the key is from the removed table, if not we skip.
-          if (table.name !== fk.referencedTableName) {
+          if (tableName !== fk.referencedTableName) {
             continue;
           }
 

packages/knex/src/schema/SchemaGenerator.ts

@@ -61,7 +61,7 @@ export class SchemaGenerator {
 
   getTargetSchema(schema?: string): DatabaseSchema {
     const metadata = this.getOrderedMetadata(schema);
-    return DatabaseSchema.fromMetadata(metadata, this.platform, this.config, schema);
+    return DatabaseSchema.fromMetadata(metadata, this.platform, this.config, schema ?? this.platform.getDefaultSchemaName());
   }
 
   async getCreateSchemaSQL(options: { wrap?: boolean; schema?: string } = {}): Promise<string> {
@@ -70,6 +70,10 @@ export class SchemaGenerator {
     let ret = '';
 
     for (const namespace of toSchema.getNamespaces()) {
+      if (namespace === this.platform.getDefaultSchemaName()) {
+        continue;
+      }
+
       ret += await this.dump(this.knex.schema.createSchemaIfNotExists(namespace));
     }
 
@@ -78,7 +82,7 @@ export class SchemaGenerator {
     }
 
     for (const tableDef of toSchema.getTables()) {
-      ret += await this.dump(this.knex.schema.withSchema(tableDef.schema!).alterTable(tableDef.name, table => this.createForeignKeys(table, tableDef, options.schema)));
+      ret += await this.dump(this.createSchemaBuilder(tableDef.schema).alterTable(tableDef.name, table => this.createForeignKeys(table, tableDef, options.schema)));
     }
 
     return this.wrapSchema(ret, { wrap });
@@ -160,13 +164,17 @@ export class SchemaGenerator {
 
     if (this.platform.supportsSchemas()) {
       for (const newNamespace of schemaDiff.newNamespaces) {
-        ret += await this.dump(this.knex.schema.createSchema(newNamespace));
+        // schema might already exist, e.g. explicit usage of `public` in postgres
+        ret += await this.dump(this.knex.schema.createSchemaIfNotExists(newNamespace));
       }
     }
 
     if (!options.safe) {
       for (const orphanedForeignKey of schemaDiff.orphanedForeignKeys) {
-        ret += await this.dump(this.knex.schema.alterTable(orphanedForeignKey.localTableName, table => table.dropForeign(orphanedForeignKey.columnNames, orphanedForeignKey.constraintName)));
+        const [schemaName, tableName] = this.splitTableName(orphanedForeignKey.localTableName);
+        ret += await this.dump(this.createSchemaBuilder(schemaName).alterTable(tableName, table => {
+          return table.dropForeign(orphanedForeignKey.columnNames, orphanedForeignKey.constraintName);
+        }));
       }
     }
 
@@ -175,7 +183,9 @@ export class SchemaGenerator {
     }
 
     for (const newTable of Object.values(schemaDiff.newTables)) {
-      ret += await this.dump(this.knex.schema.withSchema(newTable.schema!).alterTable(newTable.name, table => this.createForeignKeys(table, newTable, options.schema)));
+      ret += await this.dump(this.createSchemaBuilder(newTable.schema).alterTable(newTable.name, table => {
+        this.createForeignKeys(table, newTable, options.schema);
+      }));
     }
 
     if (options.dropTables && !options.safe) {
@@ -206,14 +216,19 @@ export class SchemaGenerator {
   }
 
   private getReferencedTableName(referencedTableName: string, schema?: string) {
-    schema ??= this.config.get('schema');
+    const [schemaName, tableName] = this.splitTableName(referencedTableName);
+    schema = schemaName ?? schema ?? this.config.get('schema');
 
     /* istanbul ignore next */
-    if (schema && referencedTableName.startsWith('*.')) {
+    if (schema && schemaName ==='*') {
       return `${schema}.${referencedTableName.replace(/^\*\./, '')}`;
     }
 
-    return referencedTableName;
+    if (schemaName === this.platform.getDefaultSchemaName()) {
+      return tableName;
+    }
+
+    return `${schemaName}.${tableName}`;
   }
 
   private createForeignKey(table: Knex.CreateTableBuilder, foreignKey: ForeignKey, schema?: string) {
@@ -239,8 +254,9 @@ export class SchemaGenerator {
     const ret: Knex.SchemaBuilder[] = [];
     const push = (sql: string) => sql ? ret.push(this.knex.schema.raw(sql)) : undefined;
     push(this.helper.getPreAlterTable(diff, safe));
+    const [schemaName, tableName] = this.splitTableName(diff.name);
 
-    ret.push(this.knex.schema.alterTable(diff.name, table => {
+    ret.push(this.createSchemaBuilder(schemaName).alterTable(tableName, table => {
       for (const foreignKey of Object.values(diff.removedForeignKeys)) {
         table.dropForeign(foreignKey.columnNames, foreignKey.constraintName);
       }
@@ -253,10 +269,19 @@ export class SchemaGenerator {
     return ret;
   }
 
+  private splitTableName(name: string): [string | undefined, string] {
+    const parts = name.split('.');
+    const tableName = parts.pop()!;
+    const schemaName = parts.pop();
+
+    return [schemaName, tableName];
+  }
+
   private alterTable(diff: TableDifference, safe: boolean): Knex.SchemaBuilder[] {
     const ret: Knex.SchemaBuilder[] = [];
+    const [schemaName, tableName] = this.splitTableName(diff.name);
 
-    ret.push(this.knex.schema.alterTable(diff.name, table => {
+    ret.push(this.createSchemaBuilder(schemaName).alterTable(tableName, table => {
       for (const index of Object.values(diff.removedIndexes)) {
         this.dropIndex(table, index);
       }
@@ -297,19 +322,19 @@ export class SchemaGenerator {
       }
 
       for (const { column } of Object.values(diff.changedColumns).filter(diff => diff.changedProperties.has('autoincrement'))) {
-        this.helper.pushTableQuery(table, this.helper.getAlterColumnAutoincrement(diff.name, column));
+        this.helper.pushTableQuery(table, this.helper.getAlterColumnAutoincrement(tableName, column));
       }
 
       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(diff.name, column));
+        this.helper.pushTableQuery(table, this.helper.getChangeColumnCommentSQL(tableName, column));
       }
 
       for (const [oldColumnName, column] of Object.entries(diff.renamedColumns)) {
-        this.helper.pushTableQuery(table, this.helper.getRenameColumnSQL(diff.name, oldColumnName, column));
+        this.helper.pushTableQuery(table, this.helper.getRenameColumnSQL(tableName, oldColumnName, column));
       }
 
       for (const foreignKey of Object.values(diff.addedForeignKeys)) {
@@ -383,8 +408,18 @@ export class SchemaGenerator {
     return ret;
   }
 
+  private createSchemaBuilder(schema?: string): Knex.SchemaBuilder {
+    const builder = this.knex.schema;
+
+    if (schema && schema !== this.platform.getDefaultSchemaName()) {
+      builder.withSchema(schema);
+    }
+
+    return builder;
+  }
+
   private createTable(tableDef: DatabaseTable): Knex.SchemaBuilder {
-    return this.knex.schema.withSchema(tableDef.schema!).createTable(tableDef.name, table => {
+    return this.createSchemaBuilder(tableDef.schema).createTable(tableDef.name, table => {
       tableDef.getColumns().forEach(column => {
         const col = this.helper.createTableColumn(table, column, tableDef);
         this.configureColumn(column, col);
@@ -418,7 +453,7 @@ export class SchemaGenerator {
       const keyName = tableName.includes('.') ? tableName.split('.').pop()! + '_pkey' : undefined;
       table.primary(index.columnNames, keyName);
     } else if (index.unique) {
-      table.unique(index.columnNames, index.keyName);
+      table.unique(index.columnNames, { indexName: index.keyName });
     } else if (index.expression) {
       this.helper.pushTableQuery(table, index.expression);
     } else {
@@ -437,11 +472,7 @@ export class SchemaGenerator {
   }
 
   private dropTable(name: string, schema?: string): Knex.SchemaBuilder {
-    let builder = this.knex.schema.dropTableIfExists(name);
-
-    if (schema) {
-      builder.withSchema(schema);
-    }
+    let builder = this.createSchemaBuilder(schema).dropTableIfExists(name);
 
     if (this.platform.usesCascadeStatement()) {
       builder = this.knex.schema.raw(builder.toQuery() + ' cascade');

packages/knex/src/schema/SchemaHelper.ts

@@ -53,7 +53,11 @@ export abstract class SchemaHelper {
   }
 
   getRenameColumnSQL(tableName: string, oldColumnName: string, to: Column): string {
-    return `alter table ${this.platform.quoteIdentifier(tableName)} rename column ${this.platform.quoteIdentifier(oldColumnName)} to ${this.platform.quoteIdentifier(to.name)}`;
+    tableName = this.platform.quoteIdentifier(tableName);
+    oldColumnName = this.platform.quoteIdentifier(oldColumnName);
+    const columnName = this.platform.quoteIdentifier(to.name);
+
+    return `alter table ${tableName} rename column ${oldColumnName} to ${columnName}`;
   }
 
   getCreateIndexSQL(tableName: string, index: Index): string {

packages/migrations/src/Migrator.ts

@@ -139,14 +139,17 @@ export class Migrator implements IMigrator {
 
     Object.values(this.em.getMetadata().getAll())
       .filter(meta => meta.collection)
-      .forEach(meta => expected.add(meta.collection));
+      .forEach(meta => {
+        const schema = meta.schema ?? this.em.getPlatform().getDefaultSchemaName();
+        expected.add(schema ? `${schema}.${meta.collection}` : meta.collection);
+      });
 
     schema.getTables().forEach(table => {
-      /* istanbul ignore next */
-      const tableName = table.schema ? `${table.schema}.${table.name}` : table.name;
+      const schema = table.schema ?? this.em.getPlatform().getDefaultSchemaName();
+      const tableName = schema ? `${schema}.${table.name}` : table.name;
 
       if (expected.has(tableName)) {
-        exists.add(tableName);
+        exists.add(table.schema ? `${table.schema}.${table.name}` : table.name);
       }
     });
 

packages/postgresql/src/PostgreSqlConnection.ts

@@ -67,7 +67,7 @@ export class PostgreSqlConnection extends AbstractSqlConnection {
     const quotedTableName = this.tableName();
     const type = col.getColumnType();
     const colName = this.client.wrapIdentifier(col.getColumnName(), col.columnBuilder.queryContext());
-    const constraintName = `${this.tableNameRaw}_${col.getColumnName()}_check`;
+    const constraintName = `${this.tableNameRaw.replace(/^.*\.(.*)$/, '$1')}_${col.getColumnName()}_check`;
     this.pushQuery({ sql: `alter table ${quotedTableName} drop constraint if exists "${constraintName}"`, bindings: [] });
     that.dropColumnDefault.call(this, col, colName);