Skip to content

Commit

Permalink
fix: resolve issues for mssql migration when simple-enum was changed
Browse files Browse the repository at this point in the history
- Changes are now detected
- Incorrect update Statement was split into a DROP CONSTRAINT and ADD CONSTRAINT Statement

Closes: typeorm#7785 typeorm#9457
  • Loading branch information
ertl committed Feb 10, 2023
1 parent 58fc088 commit 024da43
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 8 deletions.
8 changes: 7 additions & 1 deletion src/driver/sqlserver/SqlServerDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,13 @@ export class SqlServerDriver implements Driver {
tableColumn.isNullable !== columnMetadata.isNullable ||
tableColumn.asExpression !== columnMetadata.asExpression ||
tableColumn.generatedType !== columnMetadata.generatedType ||
tableColumn.isUnique !== this.normalizeIsUnique(columnMetadata)
tableColumn.isUnique !== this.normalizeIsUnique(columnMetadata) ||
(tableColumn.enum &&
columnMetadata.enum &&
!OrmUtils.isArraysEqual(
tableColumn.enum,
columnMetadata.enum.map((val) => val + ""),
))

// DEBUG SECTION
// if (isColumnChanged) {
Expand Down
46 changes: 40 additions & 6 deletions src/driver/sqlserver/SqlServerQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1566,7 +1566,7 @@ export class SqlServerQueryRunner
oldColumn.name = newColumn.name
}

if (this.isColumnChanged(oldColumn, newColumn, false)) {
if (this.isColumnChanged(oldColumn, newColumn, false, false, false)) {
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(
Expand All @@ -1576,6 +1576,7 @@ export class SqlServerQueryRunner
newColumn,
true,
false,
true
)}`,
),
)
Expand All @@ -1588,11 +1589,35 @@ export class SqlServerQueryRunner
oldColumn,
true,
false,
true
)}`,
),
)
}

if (this.isEnumChanged(oldColumn, newColumn)) {
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} DROP CONSTRAINT "${oldColumn.enumName}"`,
),
);
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} ADD CONSTRAINT "${oldColumn.enumName}" CHECK ( ${this.getEnumExpression(newColumn)} )`,
),
)
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} DROP CONSTRAINT "${oldColumn.enumName}"`,
),
);
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} ADD CONSTRAINT "${oldColumn.enumName}" CHECK ( ${this.getEnumExpression(oldColumn)} )`,
),
)
}

if (newColumn.isPrimary !== oldColumn.isPrimary) {
const primaryColumns = clonedTable.primaryColumns

Expand Down Expand Up @@ -3166,6 +3191,7 @@ export class SqlServerQueryRunner
result[1],
)
}
tableColumn.enumName=checkConstraint["CONSTRAINT_NAME"]
// Skip other column constraints
break
}
Expand Down Expand Up @@ -3904,17 +3930,15 @@ export class SqlServerQueryRunner
column: TableColumn,
skipIdentity: boolean,
createDefault: boolean,
skipEnum?: boolean
) {
let c = `"${column.name}" ${this.connection.driver.createFullType(
column,
)}`

if (column.enum) {
if (!skipEnum && column.enum) {
const expression =
column.name +
" IN (" +
column.enum.map((val) => "'" + val + "'").join(",") +
")"
this.getEnumExpression(column)
const checkName =
this.connection.namingStrategy.checkConstraintName(
table,
Expand Down Expand Up @@ -3976,6 +4000,16 @@ export class SqlServerQueryRunner
return c
}

private getEnumExpression(column: TableColumn) {
if (!column?.enum) {
throw new Error('Enum not defined!');
}
return column.name +
" IN (" +
column.enum.map((val) => "'" + val + "'").join(",") +
")";
}

protected isEnumCheckConstraint(name: string): boolean {
return name.indexOf("CHK_") !== -1 && name.indexOf("_ENUM") !== -1
}
Expand Down
7 changes: 6 additions & 1 deletion src/query-runner/BaseQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ export abstract class BaseQueryRunner {
newColumn: TableColumn,
checkDefault?: boolean,
checkComment?: boolean,
checkEnum =true
): boolean {
// this logs need to debug issues in column change detection. Do not delete it!

Expand Down Expand Up @@ -513,10 +514,14 @@ export abstract class BaseQueryRunner {
oldColumn.onUpdate !== newColumn.onUpdate || // MySQL only
oldColumn.isNullable !== newColumn.isNullable ||
(checkComment && oldColumn.comment !== newColumn.comment) ||
!OrmUtils.isArraysEqual(oldColumn.enum || [], newColumn.enum || [])
(checkEnum && this.isEnumChanged(oldColumn, newColumn))
)
}

protected isEnumChanged(oldColumn: TableColumn,newColumn: TableColumn,){
return !OrmUtils.isArraysEqual(oldColumn.enum || [], newColumn.enum || []);
}

/**
* Checks if column length is by default.
*/
Expand Down
25 changes: 25 additions & 0 deletions test/github-issues/9457/entity/ExampleEntity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Entity } from "../../../../src/decorator/entity/Entity"
import { Column } from "../../../../src/decorator/columns/Column"
import { PrimaryGeneratedColumn } from "../../../../src/decorator/columns/PrimaryGeneratedColumn"
import {Generated} from "../../../../src";

export enum ExampleEnum {
EnumValue1 = "enumvalue1",
EnumValue2 = "enumvalue2",
EnumValue3 = "enumvalue3",
EnumValue4 = "enumvalue4",
}

@Entity()
export class ExampleEntity {
@Generated("increment")
@PrimaryGeneratedColumn()
id: number

@Column({
length: 255,
type: "simple-enum",
enum: ExampleEnum,
})
enumcolumn: ExampleEnum
}
43 changes: 43 additions & 0 deletions test/github-issues/9457/issue-9457.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import "reflect-metadata"
import {
createTestingConnections,
closeTestingConnections,
} from "../../utils/test-utils"
import {DataSource} from "../../../src/data-source/DataSource"
import {expect} from "chai";

describe("github issues > #9457 No changes in database schema were found, when simple-enum is changed.", () => {
let dataSources: DataSource[]
before(
async () =>
(dataSources = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
migrations: [__dirname + "/migration/*{.js,.ts}"],
schemaCreate: false,
dropSchema: true,
enabledDrivers: ["mssql"]
})),
)
after(() => closeTestingConnections(dataSources))

it("should drop and recreate 'CHECK' constraint to match enum values", async () => {
await Promise.all(
dataSources.map(async (dataSource) => {
await dataSource.runMigrations()

const sqlInMemory = await dataSource.driver
.createSchemaBuilder()
.log()

expect(sqlInMemory.upQueries.length).to.eql(2);
expect(sqlInMemory.upQueries[0].query).to.eql('ALTER TABLE "example_entity" DROP CONSTRAINT "CHK_be8ed063b3976da24df4213baf_ENUM"')
expect(sqlInMemory.upQueries[1].query).to.eql(`ALTER TABLE "example_entity" ADD CONSTRAINT "CHK_be8ed063b3976da24df4213baf_ENUM" CHECK ( enumcolumn IN ('enumvalue1','enumvalue2','enumvalue3','enumvalue4') )`)

expect(sqlInMemory.downQueries.length).to.eql(2);
expect(sqlInMemory.downQueries[0].query).to.eql('ALTER TABLE "example_entity" DROP CONSTRAINT "CHK_be8ed063b3976da24df4213baf_ENUM"')
expect(sqlInMemory.downQueries[1].query).to.eql(`ALTER TABLE "example_entity" ADD CONSTRAINT "CHK_be8ed063b3976da24df4213baf_ENUM" CHECK ( enumcolumn IN ('enumvalue1','enumvalue2','enumvalue3') )`)
}))
})

// you can add additional tests if needed
})
20 changes: 20 additions & 0 deletions test/github-issues/9457/migration/1676011161422-init.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {MigrationInterface, QueryRunner} from "../../../../src"

export class init1676011161422 implements MigrationInterface {
name = "init1676011161422"

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`CREATE TABLE "example_entity"
(
"id" int NOT NULL IDENTITY(1,1),
"enumcolumn" nvarchar(255) CONSTRAINT CHK_be8ed063b3976da24df4213baf_ENUM CHECK (enumcolumn IN ('enumvalue1','enumvalue2','enumvalue3')) NOT NULL,
CONSTRAINT "PK_fccd73330168066a434dbac114f" PRIMARY KEY ("id")
)`,
)
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP TABLE "example_entity"`)
}
}

0 comments on commit 024da43

Please sign in to comment.