From 29cb89123aaf705437927a8c6ed23204422b71cc Mon Sep 17 00:00:00 2001 From: AlexMesser Date: Mon, 15 Nov 2021 02:45:35 +0500 Subject: [PATCH] fix: incorrect composite `UNIQUE` constraints detection (#8364) * added fix and test for #8158 * fixed for other drivers * fixed column length for Oracle --- .../AuroraDataApiQueryRunner.ts | 36 +++++++++---------- .../cockroachdb/CockroachQueryRunner.ts | 10 +++--- src/driver/mysql/MysqlQueryRunner.ts | 36 +++++++++---------- src/driver/oracle/OracleDriver.ts | 2 +- src/driver/oracle/OracleQueryRunner.ts | 22 ++++++------ src/driver/postgres/PostgresQueryRunner.ts | 14 ++++---- src/driver/sap/SapQueryRunner.ts | 35 +++++++++--------- src/driver/sqlserver/SqlServerQueryRunner.ts | 10 +++--- test/github-issues/8158/entity/User.ts | 27 ++++++++++++++ test/github-issues/8158/entity/UserMeta.ts | 21 +++++++++++ test/github-issues/8158/issue-8158.ts | 30 ++++++++++++++++ 11 files changed, 157 insertions(+), 86 deletions(-) create mode 100644 test/github-issues/8158/entity/User.ts create mode 100644 test/github-issues/8158/entity/UserMeta.ts create mode 100644 test/github-issues/8158/issue-8158.ts diff --git a/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts b/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts index 3dbaf14e79..76b0d1dc80 100644 --- a/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts +++ b/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts @@ -1270,27 +1270,25 @@ export class AuroraDataApiQueryRunner extends BaseQueryRunner implements QueryRu .filter(dbColumn => dbColumn["TABLE_NAME"] === dbTable["TABLE_NAME"] && dbColumn["TABLE_SCHEMA"] === dbTable["TABLE_SCHEMA"]) .map(dbColumn => { - const columnUniqueIndex = dbIndices.find(dbIndex => { - if (dbIndex["TABLE_NAME"] !== dbTable["TABLE_NAME"] || dbIndex["TABLE_SCHEMA"] !== dbTable["TABLE_SCHEMA"]) { - return false; - } - - // Index is not for this column - if (dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) { - return false; - } - - const nonUnique = parseInt(dbIndex["NON_UNIQUE"], 10); - return nonUnique === 0; - }); + const columnUniqueIndices = dbIndices.filter(dbIndex => { + return dbIndex["TABLE_NAME"] === dbTable["TABLE_NAME"] + && dbIndex["TABLE_SCHEMA"] === dbTable["TABLE_SCHEMA"] + && dbIndex["COLUMN_NAME"] === dbColumn["COLUMN_NAME"] + && parseInt(dbIndex["NON_UNIQUE"], 10) === 0 + }) const tableMetadata = this.connection.entityMetadatas.find(metadata => this.getTablePath(table) === this.getTablePath(metadata)); - const hasIgnoredIndex = columnUniqueIndex && tableMetadata && tableMetadata.indices - .some(index => index.name === columnUniqueIndex["INDEX_NAME"] && index.synchronize === false); + const hasIgnoredIndex = columnUniqueIndices.length > 0 + && tableMetadata + && tableMetadata.indices.some(index => { + return columnUniqueIndices.some(uniqueIndex => { + return index.name === uniqueIndex["INDEX_NAME"] && index.synchronize === false; + }); + }); - const isConstraintComposite = columnUniqueIndex - ? !!dbIndices.find(dbIndex => dbIndex["INDEX_NAME"] === columnUniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) - : false; + const isConstraintComposite = columnUniqueIndices.every((uniqueIndex) => { + return dbIndices.some(dbIndex => dbIndex["INDEX_NAME"] === uniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]); + }) const tableColumn = new TableColumn(); tableColumn.name = dbColumn["COLUMN_NAME"]; @@ -1318,7 +1316,7 @@ export class AuroraDataApiQueryRunner extends BaseQueryRunner implements QueryRu tableColumn.generatedType = dbColumn["EXTRA"].indexOf("VIRTUAL") !== -1 ? "VIRTUAL" : "STORED"; } - tableColumn.isUnique = !!columnUniqueIndex && !hasIgnoredIndex && !isConstraintComposite; + tableColumn.isUnique = columnUniqueIndices.length > 0 && !hasIgnoredIndex && !isConstraintComposite; tableColumn.isNullable = dbColumn["IS_NULLABLE"] === "YES"; tableColumn.isPrimary = dbPrimaryKeys.some(dbPrimaryKey => { return ( diff --git a/src/driver/cockroachdb/CockroachQueryRunner.ts b/src/driver/cockroachdb/CockroachQueryRunner.ts index d3c05778e6..1eff387e4a 100644 --- a/src/driver/cockroachdb/CockroachQueryRunner.ts +++ b/src/driver/cockroachdb/CockroachQueryRunner.ts @@ -1556,13 +1556,13 @@ export class CockroachQueryRunner extends BaseQueryRunner implements QueryRunner tableColumn.isNullable = dbColumn["is_nullable"] === "YES"; tableColumn.isPrimary = !!columnConstraints.find(constraint => constraint["constraint_type"] === "PRIMARY"); - const uniqueConstraint = columnConstraints.find(constraint => constraint["constraint_type"] === "UNIQUE"); - const isConstraintComposite = uniqueConstraint - ? !!dbConstraints.find(dbConstraint => dbConstraint["constraint_type"] === "UNIQUE" + const uniqueConstraints = columnConstraints.filter(constraint => constraint["constraint_type"] === "UNIQUE"); + const isConstraintComposite = uniqueConstraints.every((uniqueConstraint) => { + return dbConstraints.some(dbConstraint => dbConstraint["constraint_type"] === "UNIQUE" && dbConstraint["constraint_name"] === uniqueConstraint["constraint_name"] && dbConstraint["column_name"] !== dbColumn["column_name"]) - : false; - tableColumn.isUnique = !!uniqueConstraint && !isConstraintComposite; + }) + tableColumn.isUnique = uniqueConstraints.length > 0 && !isConstraintComposite; if (dbColumn["column_default"] !== null && dbColumn["column_default"] !== undefined) { if (dbColumn["column_default"] === "unique_rowid()") { diff --git a/src/driver/mysql/MysqlQueryRunner.ts b/src/driver/mysql/MysqlQueryRunner.ts index c906f730e6..c6f99dd0c6 100644 --- a/src/driver/mysql/MysqlQueryRunner.ts +++ b/src/driver/mysql/MysqlQueryRunner.ts @@ -1451,27 +1451,25 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { .filter(dbColumn => dbColumn["TABLE_NAME"] === dbTable["TABLE_NAME"] && dbColumn["TABLE_SCHEMA"] === dbTable["TABLE_SCHEMA"]) .map(dbColumn => { - const columnUniqueIndex = dbIndices.find(dbIndex => { - if (dbIndex["TABLE_NAME"] !== dbTable["TABLE_NAME"] || dbIndex["TABLE_SCHEMA"] !== dbTable["TABLE_SCHEMA"]) { - return false; - } - - // Index is not for this column - if (dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) { - return false; - } - - const nonUnique = parseInt(dbIndex["NON_UNIQUE"], 10); - return nonUnique === 0; - }); + const columnUniqueIndices = dbIndices.filter(dbIndex => { + return dbIndex["TABLE_NAME"] === dbTable["TABLE_NAME"] + && dbIndex["TABLE_SCHEMA"] === dbTable["TABLE_SCHEMA"] + && dbIndex["COLUMN_NAME"] === dbColumn["COLUMN_NAME"] + && parseInt(dbIndex["NON_UNIQUE"], 10) === 0 + }) const tableMetadata = this.connection.entityMetadatas.find(metadata => this.getTablePath(table) === this.getTablePath(metadata)); - const hasIgnoredIndex = columnUniqueIndex && tableMetadata && tableMetadata.indices - .some(index => index.name === columnUniqueIndex["INDEX_NAME"] && index.synchronize === false); + const hasIgnoredIndex = columnUniqueIndices.length > 0 + && tableMetadata + && tableMetadata.indices.some(index => { + return columnUniqueIndices.some(uniqueIndex => { + return index.name === uniqueIndex["INDEX_NAME"] && index.synchronize === false; + }); + }); - const isConstraintComposite = columnUniqueIndex - ? !!dbIndices.find(dbIndex => dbIndex["INDEX_NAME"] === columnUniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) - : false; + const isConstraintComposite = columnUniqueIndices.every((uniqueIndex) => { + return dbIndices.some(dbIndex => dbIndex["INDEX_NAME"] === uniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]); + }) const tableColumn = new TableColumn(); tableColumn.name = dbColumn["COLUMN_NAME"]; @@ -1512,7 +1510,7 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { tableColumn.generatedType = dbColumn["EXTRA"].indexOf("VIRTUAL") !== -1 ? "VIRTUAL" : "STORED"; } - tableColumn.isUnique = !!columnUniqueIndex && !hasIgnoredIndex && !isConstraintComposite; + tableColumn.isUnique = columnUniqueIndices.length > 0 && !hasIgnoredIndex && !isConstraintComposite; tableColumn.isNullable = dbColumn["IS_NULLABLE"] === "YES"; tableColumn.isPrimary = dbPrimaryKeys.some(dbPrimaryKey => { return ( diff --git a/src/driver/oracle/OracleDriver.ts b/src/driver/oracle/OracleDriver.ts index 8fb6bba649..95402e8d0e 100644 --- a/src/driver/oracle/OracleDriver.ts +++ b/src/driver/oracle/OracleDriver.ts @@ -697,7 +697,7 @@ export class OracleDriver implements Driver { const isColumnChanged = tableColumn.name !== columnMetadata.databaseName || tableColumn.type !== this.normalizeType(columnMetadata) - || tableColumn.length !== columnMetadata.length + || tableColumn.length !== this.getColumnLength(columnMetadata) || tableColumn.precision !== columnMetadata.precision || tableColumn.scale !== columnMetadata.scale // || tableColumn.comment !== columnMetadata.comment diff --git a/src/driver/oracle/OracleQueryRunner.ts b/src/driver/oracle/OracleQueryRunner.ts index 1df251734c..0d94be360c 100644 --- a/src/driver/oracle/OracleQueryRunner.ts +++ b/src/driver/oracle/OracleQueryRunner.ts @@ -1367,18 +1367,16 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner { ) ); - const uniqueConstraint = columnConstraints.find(constraint => constraint["CONSTRAINT_TYPE"] === "U"); - const isConstraintComposite = uniqueConstraint - ? !!dbConstraints.find(dbConstraint => ( - dbConstraint["OWNER"] === dbColumn["OWNER"] && - dbConstraint["TABLE_NAME"] === dbColumn["TABLE_NAME"] && - dbConstraint["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"] && - dbConstraint["CONSTRAINT_NAME"] === uniqueConstraint["CONSTRAINT_NAME"] && - dbConstraint["CONSTRAINT_TYPE"] === "U" - ) + const uniqueConstraints = columnConstraints.filter(constraint => constraint["CONSTRAINT_TYPE"] === "U"); + const isConstraintComposite = uniqueConstraints.every((uniqueConstraint) => { + return dbConstraints.some(dbConstraint => ( + dbConstraint["OWNER"] === dbColumn["OWNER"] && + dbConstraint["TABLE_NAME"] === dbColumn["TABLE_NAME"] && + dbConstraint["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"] && + dbConstraint["CONSTRAINT_NAME"] === uniqueConstraint["CONSTRAINT_NAME"] && + dbConstraint["CONSTRAINT_TYPE"] === "U") ) - : false; - const isUnique = !!uniqueConstraint && !isConstraintComposite; + }) const isPrimary = !!columnConstraints.find(constraint => constraint["CONSTRAINT_TYPE"] === "P"); @@ -1411,7 +1409,7 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner { && dbColumn["DATA_DEFAULT"].trim() !== "NULL" ? tableColumn.default = dbColumn["DATA_DEFAULT"].trim() : undefined; tableColumn.isNullable = dbColumn["NULLABLE"] === "Y"; - tableColumn.isUnique = isUnique; + tableColumn.isUnique = uniqueConstraints.length > 0 && !isConstraintComposite; tableColumn.isPrimary = isPrimary; tableColumn.isGenerated = dbColumn["IDENTITY_COLUMN"] === "YES"; if (tableColumn.isGenerated) { diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts index 4a0013ae14..fd096234d8 100644 --- a/src/driver/postgres/PostgresQueryRunner.ts +++ b/src/driver/postgres/PostgresQueryRunner.ts @@ -1901,13 +1901,13 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner tableColumn.isNullable = dbColumn["is_nullable"] === "YES"; tableColumn.isPrimary = !!columnConstraints.find(constraint => constraint["constraint_type"] === "PRIMARY"); - const uniqueConstraint = columnConstraints.find(constraint => constraint["constraint_type"] === "UNIQUE"); - const isConstraintComposite = uniqueConstraint - ? !!dbConstraints.find(dbConstraint => dbConstraint["constraint_type"] === "UNIQUE" - && dbConstraint["constraint_name"] === uniqueConstraint["constraint_name"] - && dbConstraint["column_name"] !== dbColumn["column_name"]) - : false; - tableColumn.isUnique = !!uniqueConstraint && !isConstraintComposite; + const uniqueConstraints = columnConstraints.filter(constraint => constraint["constraint_type"] === "UNIQUE"); + const isConstraintComposite = uniqueConstraints.every((uniqueConstraint) => { + return dbConstraints.some(dbConstraint => dbConstraint["constraint_type"] === "UNIQUE" + && dbConstraint["constraint_name"] === uniqueConstraint["constraint_name"] + && dbConstraint["column_name"] !== dbColumn["column_name"]) + }) + tableColumn.isUnique = uniqueConstraints.length > 0 && !isConstraintComposite; if (dbColumn.is_identity === "YES") { // Postgres 10+ Identity column tableColumn.isGenerated = true; diff --git a/src/driver/sap/SapQueryRunner.ts b/src/driver/sap/SapQueryRunner.ts index a3d4fc07a6..3d1d039168 100644 --- a/src/driver/sap/SapQueryRunner.ts +++ b/src/driver/sap/SapQueryRunner.ts @@ -1588,26 +1588,25 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner { dbConstraint["COLUMN_NAME"] === dbColumn["COLUMN_NAME"] )); - const columnUniqueIndex = dbIndices.find(dbIndex => { - if (dbIndex["TABLE_NAME"] !== dbTable["TABLE_NAME"] || dbIndex["SCHEMA_NAME"] !== dbTable["SCHEMA_NAME"]) { - return false; - } - - // Index is not for this column - if (dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) { - return false; - } - - return dbIndex["CONSTRAINT"] && dbIndex["CONSTRAINT"].indexOf("UNIQUE") !== -1; - }); + const columnUniqueIndices = dbIndices.filter(dbIndex => { + return dbIndex["TABLE_NAME"] === dbTable["TABLE_NAME"] + && dbIndex["SCHEMA_NAME"] === dbTable["SCHEMA_NAME"] + && dbIndex["COLUMN_NAME"] === dbColumn["COLUMN_NAME"] + && dbIndex["CONSTRAINT"] && dbIndex["CONSTRAINT"].indexOf("UNIQUE") !== -1 + }) const tableMetadata = this.connection.entityMetadatas.find(metadata => this.getTablePath(table) === this.getTablePath(metadata)); - const hasIgnoredIndex = columnUniqueIndex && tableMetadata && tableMetadata.indices - .some(index => index.name === columnUniqueIndex["INDEX_NAME"] && index.synchronize === false); + const hasIgnoredIndex = columnUniqueIndices.length > 0 + && tableMetadata + && tableMetadata.indices.some(index => { + return columnUniqueIndices.some(uniqueIndex => { + return index.name === uniqueIndex["INDEX_NAME"] && index.synchronize === false; + }) + }); - const isConstraintComposite = columnUniqueIndex - ? !!dbIndices.find(dbIndex => dbIndex["INDEX_NAME"] === columnUniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) - : false; + const isConstraintComposite = columnUniqueIndices.every((uniqueIndex) => { + return dbIndices.some(dbIndex => dbIndex["INDEX_NAME"] === uniqueIndex["INDEX_NAME"] && dbIndex["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]); + }) const tableColumn = new TableColumn(); tableColumn.name = dbColumn["COLUMN_NAME"]; @@ -1638,7 +1637,7 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner { const length = dbColumn["LENGTH"].toString(); tableColumn.length = !this.isDefaultColumnLength(table, tableColumn, length) ? length : ""; } - tableColumn.isUnique = !!columnUniqueIndex && !hasIgnoredIndex && !isConstraintComposite; + tableColumn.isUnique = columnUniqueIndices.length > 0 && !hasIgnoredIndex && !isConstraintComposite; tableColumn.isNullable = dbColumn["IS_NULLABLE"] === "TRUE"; tableColumn.isPrimary = !!columnConstraints.find(constraint => constraint["IS_PRIMARY_KEY"] === "TRUE"); tableColumn.isGenerated = dbColumn["GENERATION_TYPE"] === "ALWAYS AS IDENTITY"; diff --git a/src/driver/sqlserver/SqlServerQueryRunner.ts b/src/driver/sqlserver/SqlServerQueryRunner.ts index 7146f1bde9..bc78b51e85 100644 --- a/src/driver/sqlserver/SqlServerQueryRunner.ts +++ b/src/driver/sqlserver/SqlServerQueryRunner.ts @@ -1745,14 +1745,14 @@ export class SqlServerQueryRunner extends BaseQueryRunner implements QueryRunner dbConstraint["COLUMN_NAME"] === dbColumn["COLUMN_NAME"] )); - const uniqueConstraint = columnConstraints.find(constraint => constraint["CONSTRAINT_TYPE"] === "UNIQUE"); - const isConstraintComposite = uniqueConstraint - ? !!dbConstraints.find(dbConstraint => dbConstraint["CONSTRAINT_TYPE"] === "UNIQUE" + const uniqueConstraints = columnConstraints.filter(constraint => constraint["CONSTRAINT_TYPE"] === "UNIQUE"); + const isConstraintComposite = uniqueConstraints.every((uniqueConstraint) => { + return dbConstraints.some(dbConstraint => dbConstraint["CONSTRAINT_TYPE"] === "UNIQUE" && dbConstraint["CONSTRAINT_NAME"] === uniqueConstraint["CONSTRAINT_NAME"] && dbConstraint["TABLE_SCHEMA"] === dbColumn["TABLE_SCHEMA"] && dbConstraint["TABLE_CATALOG"] === dbColumn["TABLE_CATALOG"] && dbConstraint["COLUMN_NAME"] !== dbColumn["COLUMN_NAME"]) - : false; + }) const isPrimary = !!columnConstraints.find(constraint => constraint["CONSTRAINT_TYPE"] === "PRIMARY KEY"); const isGenerated = !!dbIdentityColumns.find(column => ( @@ -1809,7 +1809,7 @@ export class SqlServerQueryRunner extends BaseQueryRunner implements QueryRunner : undefined; tableColumn.isNullable = dbColumn["IS_NULLABLE"] === "YES"; tableColumn.isPrimary = isPrimary; - tableColumn.isUnique = !!uniqueConstraint && !isConstraintComposite; + tableColumn.isUnique = uniqueConstraints.length > 0 && !isConstraintComposite; tableColumn.isGenerated = isGenerated; if (isGenerated) tableColumn.generationStrategy = "increment"; diff --git a/test/github-issues/8158/entity/User.ts b/test/github-issues/8158/entity/User.ts new file mode 100644 index 0000000000..daf7070207 --- /dev/null +++ b/test/github-issues/8158/entity/User.ts @@ -0,0 +1,27 @@ +import {Column, Entity, JoinColumn, OneToOne, PrimaryGeneratedColumn, Unique} from "../../../../src"; +import {UserMeta} from "./UserMeta"; + +@Entity() +@Unique(["firstName"]) +@Unique(["id", "firstName"]) +export class User { + @PrimaryGeneratedColumn() + id: number; + + @Column() + firstName: string; + + @Column() + lastName: string; + + @Column() + age: number; + + @Column({ nullable: false, }) + userMetaId: number; + + @OneToOne(() => UserMeta) + @JoinColumn({ name: "userMetaId", referencedColumnName: "id" }) + userMeta: UserMeta; + +} diff --git a/test/github-issues/8158/entity/UserMeta.ts b/test/github-issues/8158/entity/UserMeta.ts new file mode 100644 index 0000000000..b510259663 --- /dev/null +++ b/test/github-issues/8158/entity/UserMeta.ts @@ -0,0 +1,21 @@ +import {Column, Entity, JoinColumn, OneToOne, PrimaryGeneratedColumn} from "../../../../src"; +import {User} from "./User"; + +@Entity() +export class UserMeta { + @PrimaryGeneratedColumn() + id: number; + + @Column() + foo: number; + + @Column() + bar: number; + + @Column({ nullable: false, }) + userId: number; + + @OneToOne(() => User) + @JoinColumn({ name: "userId", referencedColumnName: "id" }) + user: User; +} diff --git a/test/github-issues/8158/issue-8158.ts b/test/github-issues/8158/issue-8158.ts new file mode 100644 index 0000000000..e99b596485 --- /dev/null +++ b/test/github-issues/8158/issue-8158.ts @@ -0,0 +1,30 @@ +import "reflect-metadata"; +import {Connection} from "../../../src"; +import {createTestingConnections, closeTestingConnections} from "../../utils/test-utils"; +import {UserMeta} from "./entity/UserMeta"; +import {User} from "./entity/User"; + +describe("github issues > #8158 Typeorm creates migration that creates already existing unique constraint", () => { + let connections: Connection[]; + before(async () => connections = await createTestingConnections({ + migrations: [], + schemaCreate: false, + dropSchema: true, + entities: [UserMeta, User], + })); + after(() => closeTestingConnections(connections)); + + it("should recognize model changes", () => Promise.all(connections.map(async connection => { + const sqlInMemory = await connection.driver.createSchemaBuilder().log(); + sqlInMemory.upQueries.length.should.be.greaterThan(0); + sqlInMemory.downQueries.length.should.be.greaterThan(0); + }))); + + it("should not generate queries when no model changes", () => Promise.all(connections.map(async connection => { + await connection.driver.createSchemaBuilder().build(); + + const sqlInMemory = await connection.driver.createSchemaBuilder().log(); + sqlInMemory.upQueries.length.should.be.equal(0); + sqlInMemory.downQueries.length.should.be.equal(0); + }))); +});