From 42083936e2b65f0d1bd8e23d12689a7f49e2da2f Mon Sep 17 00:00:00 2001 From: Ed Mitchell Date: Sat, 12 Feb 2022 09:05:33 -0600 Subject: [PATCH] fix: adding/removing @Generated() will now generate a migration to add/remove the DEFAULT value (#8274) * fix(8273) Adding @Generated('uuid') will now generate a migration to add the DEFAULT value * implemented fix for "increment" generation type; implemented fix for generation removal; improved tests; * added test for #5898 Co-authored-by: AlexMesser --- src/driver/postgres/PostgresQueryRunner.ts | 50 +++++++--- test/github-issues/5898/entity/Album.ts | 11 +++ test/github-issues/5898/entity/Document.ts | 12 +++ test/github-issues/5898/entity/Photo.ts | 12 +++ test/github-issues/5898/entity/User.ts | 11 +++ test/github-issues/5898/issue-5898.ts | 110 +++++++++++++++++++++ test/github-issues/8273/entity/User.ts | 22 +++++ test/github-issues/8273/issue-8273.ts | 107 ++++++++++++++++++++ 8 files changed, 321 insertions(+), 14 deletions(-) create mode 100644 test/github-issues/5898/entity/Album.ts create mode 100644 test/github-issues/5898/entity/Document.ts create mode 100644 test/github-issues/5898/entity/Photo.ts create mode 100644 test/github-issues/5898/entity/User.ts create mode 100644 test/github-issues/5898/issue-5898.ts create mode 100644 test/github-issues/8273/entity/User.ts create mode 100644 test/github-issues/8273/issue-8273.ts diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts index 8da402440d..69ff292459 100644 --- a/src/driver/postgres/PostgresQueryRunner.ts +++ b/src/driver/postgres/PostgresQueryRunner.ts @@ -1020,23 +1020,45 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner } } - if (oldColumn.isGenerated !== newColumn.isGenerated && - newColumn.generationStrategy !== "uuid" && - newColumn.generationStrategy !== "identity" - ) { - if (newColumn.isGenerated === true) { - upQueries.push(new Query(`CREATE SEQUENCE IF NOT EXISTS ${this.escapePath(this.buildSequencePath(table, newColumn))} OWNED BY ${this.escapePath(table)}."${newColumn.name}"`)); - downQueries.push(new Query(`DROP SEQUENCE ${this.escapePath(this.buildSequencePath(table, newColumn))}`)); + if (oldColumn.isGenerated !== newColumn.isGenerated) { + // if old column was "generated", we should clear defaults + if (oldColumn.isGenerated) { + if (oldColumn.generationStrategy === "uuid") { + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${oldColumn.name}" DROP DEFAULT`)); + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${oldColumn.name}" SET DEFAULT ${this.driver.uuidGenerator}`)) + + } else if (oldColumn.generationStrategy === "increment") { + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT nextval('${this.escapePath(this.buildSequencePath(table, newColumn))}')`)); + + upQueries.push(new Query(`DROP SEQUENCE ${this.escapePath(this.buildSequencePath(table, newColumn))}`)); + downQueries.push(new Query(`CREATE SEQUENCE IF NOT EXISTS ${this.escapePath(this.buildSequencePath(table, newColumn))} OWNED BY ${this.escapePath(table)}."${newColumn.name}"`)); + } + } - upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT nextval('${this.escapePath(this.buildSequencePath(table, newColumn))}')`)); - downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); + if (newColumn.generationStrategy === "uuid") { + if (newColumn.isGenerated === true) { + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT ${this.driver.uuidGenerator}`)) + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); + } else { + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT ${this.driver.uuidGenerator}`)) + } + } else if (newColumn.generationStrategy === "increment") { + if (newColumn.isGenerated === true) { + upQueries.push(new Query(`CREATE SEQUENCE IF NOT EXISTS ${this.escapePath(this.buildSequencePath(table, newColumn))} OWNED BY ${this.escapePath(table)}."${newColumn.name}"`)); + downQueries.push(new Query(`DROP SEQUENCE ${this.escapePath(this.buildSequencePath(table, newColumn))}`)); - } else { - upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); - downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT nextval('${this.escapePath(this.buildSequencePath(table, newColumn))}')`)); + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT nextval('${this.escapePath(this.buildSequencePath(table, newColumn))}')`)); + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); - upQueries.push(new Query(`DROP SEQUENCE ${this.escapePath(this.buildSequencePath(table, newColumn))}`)); - downQueries.push(new Query(`CREATE SEQUENCE IF NOT EXISTS ${this.escapePath(this.buildSequencePath(table, newColumn))} OWNED BY ${this.escapePath(table)}."${newColumn.name}"`)); + } else { + upQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" DROP DEFAULT`)); + downQueries.push(new Query(`ALTER TABLE ${this.escapePath(table)} ALTER COLUMN "${newColumn.name}" SET DEFAULT nextval('${this.escapePath(this.buildSequencePath(table, newColumn))}')`)); + + upQueries.push(new Query(`DROP SEQUENCE ${this.escapePath(this.buildSequencePath(table, newColumn))}`)); + downQueries.push(new Query(`CREATE SEQUENCE IF NOT EXISTS ${this.escapePath(this.buildSequencePath(table, newColumn))} OWNED BY ${this.escapePath(table)}."${newColumn.name}"`)); + } } } diff --git a/test/github-issues/5898/entity/Album.ts b/test/github-issues/5898/entity/Album.ts new file mode 100644 index 0000000000..05e3ed4957 --- /dev/null +++ b/test/github-issues/5898/entity/Album.ts @@ -0,0 +1,11 @@ +import {Entity, OneToMany, PrimaryGeneratedColumn} from "../../../../src"; +import {Photo} from "./Photo"; + +@Entity() +export class Album { + @PrimaryGeneratedColumn() + id: string + + @OneToMany(() => Photo, (photo) => photo.album, { onDelete: "CASCADE" }) + photos: Photo[] +} diff --git a/test/github-issues/5898/entity/Document.ts b/test/github-issues/5898/entity/Document.ts new file mode 100644 index 0000000000..36fa910b1e --- /dev/null +++ b/test/github-issues/5898/entity/Document.ts @@ -0,0 +1,12 @@ +import {Entity, JoinColumn, ManyToOne, PrimaryColumn} from "../../../../src"; +import {User} from "./User"; + +@Entity() +export class Document { + @PrimaryColumn("uuid") + id: string + + @ManyToOne(() => User, (user) => user.docs, {onDelete: "CASCADE"}) + @JoinColumn({name: "ownerId"}) + owner: User +} diff --git a/test/github-issues/5898/entity/Photo.ts b/test/github-issues/5898/entity/Photo.ts new file mode 100644 index 0000000000..ae0b9d6011 --- /dev/null +++ b/test/github-issues/5898/entity/Photo.ts @@ -0,0 +1,12 @@ +import {Entity, JoinColumn, ManyToOne, PrimaryColumn} from "../../../../src"; +import {Album} from "./Album"; + +@Entity() +export class Photo { + @PrimaryColumn() + id: string + + @ManyToOne(() => Album, (album) => album.photos, {onDelete: "CASCADE"}) + @JoinColumn({name: "albumId"}) + album: Album +} diff --git a/test/github-issues/5898/entity/User.ts b/test/github-issues/5898/entity/User.ts new file mode 100644 index 0000000000..eec549973c --- /dev/null +++ b/test/github-issues/5898/entity/User.ts @@ -0,0 +1,11 @@ +import {Entity, OneToMany, PrimaryGeneratedColumn} from "../../../../src"; +import {Document} from "./Document"; + +@Entity() +export class User { + @PrimaryGeneratedColumn("uuid") + id: string + + @OneToMany(() => Document, (doc) => doc.owner, { onDelete: "CASCADE" }) + docs: Document[] +} diff --git a/test/github-issues/5898/issue-5898.ts b/test/github-issues/5898/issue-5898.ts new file mode 100644 index 0000000000..fc1c0fab0d --- /dev/null +++ b/test/github-issues/5898/issue-5898.ts @@ -0,0 +1,110 @@ +import "reflect-metadata"; +import {expect} from "chai"; +import {Connection, QueryRunner} from "../../../src"; +import {createTestingConnections, closeTestingConnections} from "../../utils/test-utils"; +import {User} from "./entity/User"; +import {Document} from "./entity/Document"; +import {Album} from "./entity/Album"; +import {Photo} from "./entity/Photo"; + +describe("github issues > #5898 Postgres primary key of type uuid: default value migration/sync not working", () => { + let connections: Connection[]; + const getColumnDefault = async (queryRunner: QueryRunner, tableName: string, columnName: string): Promise => { + const query = `SELECT "column_default"` + + ` FROM "information_schema"."columns"` + + ` WHERE "table_schema" = 'public' AND "table_name" = '${tableName}' AND "column_name" = '${columnName}'` + const res = await queryRunner.query(query) + return res.length ? res[0]["column_default"] : null + } + before(async () => connections = await createTestingConnections({ + enabledDrivers: ["postgres"], + schemaCreate: true, + dropSchema: true, + entities: [User, Document, Album, Photo], + })); + after(() => closeTestingConnections(connections)); + + it("should add DEFAULT value when @PrimaryGeneratedColumn('increment') is added", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("photo"); + const column = table!.findColumnByName("id")!; + const newColumn = column.clone(); + newColumn.isGenerated = true; + newColumn.generationStrategy = "increment"; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "photo", "id") + expect(columnDefault).to.equal("nextval('photo_id_seq'::regclass)") + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "photo", "id") + expect(columnDefault).to.null + + await queryRunner.release() + }))); + + it("should remove DEFAULT value when @PrimaryGeneratedColumn('increment') is removed", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("album"); + const column = table!.findColumnByName("id")!; + const newColumn = column.clone(); + newColumn.isGenerated = false; + newColumn.generationStrategy = undefined; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "album", "id") + expect(columnDefault).to.null + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "album", "id") + expect(columnDefault).to.equal(`nextval('album_id_seq'::regclass)`) + + await queryRunner.release() + }))); + + it("should add DEFAULT value when @PrimaryGeneratedColumn('uuid') is added", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("document"); + const column = table!.findColumnByName("id")!; + const newColumn = column.clone(); + newColumn.isGenerated = true; + newColumn.generationStrategy = "uuid"; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "document", "id") + expect(columnDefault).to.equal("uuid_generate_v4()") + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "document", "id") + expect(columnDefault).to.null + + await queryRunner.release() + }))); + + it("should remove DEFAULT value when @PrimaryGeneratedColumn('uuid') is removed", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("user"); + const column = table!.findColumnByName("id")!; + const newColumn = column.clone(); + newColumn.isGenerated = false; + newColumn.generationStrategy = undefined; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "user", "id") + expect(columnDefault).to.null + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "user", "id") + expect(columnDefault).to.equal("uuid_generate_v4()") + + await queryRunner.release() + }))); +}); diff --git a/test/github-issues/8273/entity/User.ts b/test/github-issues/8273/entity/User.ts new file mode 100644 index 0000000000..22e23d29ee --- /dev/null +++ b/test/github-issues/8273/entity/User.ts @@ -0,0 +1,22 @@ +import { Entity, PrimaryColumn, Column, Generated } from "../../../../src"; + +@Entity() +export class User { + + @PrimaryColumn() + id: number; + + @Column({ type: "uuid" }) + uuid: string; + + @Column({ type: "uuid" }) + @Generated("uuid") + uuidWithGenerated: string; + + @Column() + increment: number; + + @Column() + @Generated("increment") + incrementWithGenerated: number; +} diff --git a/test/github-issues/8273/issue-8273.ts b/test/github-issues/8273/issue-8273.ts new file mode 100644 index 0000000000..212e651622 --- /dev/null +++ b/test/github-issues/8273/issue-8273.ts @@ -0,0 +1,107 @@ +import "reflect-metadata"; +import {expect} from "chai"; +import {Connection, QueryRunner} from "../../../src"; +import {createTestingConnections, closeTestingConnections} from "../../utils/test-utils"; +import {User} from "./entity/User"; + +describe("github issues > #8273 Adding @Generated('uuid') doesn't update column default in PostgreSQL", () => { + let connections: Connection[]; + const getColumnDefault = async (queryRunner: QueryRunner, columnName: string): Promise => { + const query = `SELECT "column_default"`+ + ` FROM "information_schema"."columns"`+ + ` WHERE "table_schema" = 'public' AND "table_name" = 'user' AND "column_name" = '${columnName}'` + const res = await queryRunner.query(query) + return res.length ? res[0]["column_default"] : null + } + before(async () => connections = await createTestingConnections({ + enabledDrivers: ["postgres"], + schemaCreate: true, + dropSchema: true, + entities: [User], + })); + after(() => closeTestingConnections(connections)); + + it("should add DEFAULT value when @Generated('increment') is added", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("user"); + const column = table!.findColumnByName("increment")!; + const newColumn = column.clone(); + newColumn.isGenerated = true; + newColumn.generationStrategy = "increment"; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "increment") + expect(columnDefault).to.equal("nextval('user_increment_seq'::regclass)") + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "increment") + expect(columnDefault).to.null + + await queryRunner.release() + }))); + + it("should remove DEFAULT value when @Generated('increment') is removed", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("user"); + const column = table!.findColumnByName("incrementWithGenerated")!; + const newColumn = column.clone(); + newColumn.isGenerated = false; + newColumn.generationStrategy = undefined; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "incrementWithGenerated") + expect(columnDefault).to.null + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "incrementWithGenerated") + expect(columnDefault).to.equal(`nextval('"user_incrementWithGenerated_seq"'::regclass)`) + + await queryRunner.release() + }))); + + it("should add DEFAULT value when @Generated('uuid') is added", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("user"); + const column = table!.findColumnByName("uuid")!; + const newColumn = column.clone(); + newColumn.isGenerated = true; + newColumn.generationStrategy = "uuid"; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "uuid") + expect(columnDefault).to.equal("uuid_generate_v4()") + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "uuid") + expect(columnDefault).to.null + + await queryRunner.release() + }))); + + it("should remove DEFAULT value when @Generated('uuid') is removed", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + let table = await queryRunner.getTable("user"); + const column = table!.findColumnByName("uuidWithGenerated")!; + const newColumn = column.clone(); + newColumn.isGenerated = false; + newColumn.generationStrategy = undefined; + + await queryRunner.changeColumn(table!, column, newColumn) + + let columnDefault = await getColumnDefault(queryRunner, "uuidWithGenerated") + expect(columnDefault).to.null + + await queryRunner.executeMemoryDownSql(); + + columnDefault = await getColumnDefault(queryRunner, "uuidWithGenerated") + expect(columnDefault).to.equal("uuid_generate_v4()") + + await queryRunner.release() + }))); +});