From 27e5ad6bbdb5dfc203d4032b910bf29328b9f631 Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Fri, 28 Aug 2020 22:14:28 +0200 Subject: [PATCH 1/3] fix: Migration issues with scale & precision in sqlite/sql.js Specifying precision or scale properties on columns with SQLite/sql.js would result in migrations being generated even on an unchanged schema. This was due to the precision and scale arguments not correctly being inferred when reading the table. This change handles scale and precision in the same way that "length" was already being correctly handled. Fixes #6636 --- .../sqlite-abstract/AbstractSqliteDriver.ts | 23 +++++++++++++++-- .../AbstractSqliteQueryRunner.ts | 20 ++++++++++++--- test/github-issues/6636/entity/Test.ts | 15 +++++++++++ test/github-issues/6636/issue-6636.ts | 25 +++++++++++++++++++ 4 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 test/github-issues/6636/entity/Test.ts create mode 100644 test/github-issues/6636/issue-6636.ts diff --git a/src/driver/sqlite-abstract/AbstractSqliteDriver.ts b/src/driver/sqlite-abstract/AbstractSqliteDriver.ts index 09cb044e17..dc406c7bb7 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteDriver.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteDriver.ts @@ -130,12 +130,31 @@ export abstract class AbstractSqliteDriver implements Driver { /** * Gets list of column data types that support precision by a driver. */ - withPrecisionColumnTypes: ColumnType[] = []; + withPrecisionColumnTypes: ColumnType[] = [ + "real", + "double", + "double precision", + "float", + "real", + "numeric", + "decimal", + "date", + "time", + "datetime" + ]; /** * Gets list of column data types that support scale by a driver. */ - withScaleColumnTypes: ColumnType[] = []; + withScaleColumnTypes: ColumnType[] = [ + "real", + "double", + "double precision", + "float", + "real", + "numeric", + "decimal", + ]; /** * Orm has special columns and we need to know what database column types should be for those types. diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index c361695c2c..995b22d785 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -825,17 +825,31 @@ export abstract class AbstractSqliteQueryRunner extends BaseQueryRunner implemen } } - // parse datatype and attempt to retrieve length + // parse datatype and attempt to retrieve length, precision and scale let pos = tableColumn.type.indexOf("("); if (pos !== -1) { - let dataType = tableColumn.type.substr(0, pos); + const fullType = tableColumn.type; + let dataType = fullType.substr(0, pos); if (!!this.driver.withLengthColumnTypes.find(col => col === dataType)) { - let len = parseInt(tableColumn.type.substring(pos + 1, tableColumn.type.length - 1)); + let len = parseInt(fullType.substring(pos + 1, fullType.length - 1)); if (len) { tableColumn.length = len.toString(); tableColumn.type = dataType; // remove the length part from the datatype } } + if (!!this.driver.withPrecisionColumnTypes.find(col => col === dataType)) { + const re = new RegExp(`^${dataType}\\((\\d+),?\\s?(\\d+)?\\)`); + const matches = fullType.match(re); + if (matches && matches[1]) { + tableColumn.precision = +matches[1]; + } + if (!!this.driver.withScaleColumnTypes.find(col => col === dataType)) { + if (matches && matches[2]) { + tableColumn.scale = +matches[2]; + } + } + tableColumn.type = dataType; // remove the precision/scale part from the datatype + } } return tableColumn; diff --git a/test/github-issues/6636/entity/Test.ts b/test/github-issues/6636/entity/Test.ts new file mode 100644 index 0000000000..086363bbbc --- /dev/null +++ b/test/github-issues/6636/entity/Test.ts @@ -0,0 +1,15 @@ +import { Column, Entity, PrimaryColumn } from "../../../../src"; + +@Entity() +export class Test { + + @PrimaryColumn() + id: number; + + @Column({ nullable: true, precision: 6 }) + startedAt?: Date; + + @Column({ type: 'decimal', precision: 5, scale: 2 }) + value: number; + +} diff --git a/test/github-issues/6636/issue-6636.ts b/test/github-issues/6636/issue-6636.ts new file mode 100644 index 0000000000..bf40f67275 --- /dev/null +++ b/test/github-issues/6636/issue-6636.ts @@ -0,0 +1,25 @@ +import { Connection } from "../../../src"; +import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; +import { Test } from "./entity/Test"; +import { expect } from "chai"; + +describe("github issues > #6636 migration issues with scale & precision", () => { + + let connections: Connection[]; + before(async () => connections = await createTestingConnections({ + entities: [Test], + enabledDrivers: ["sqljs", "sqlite", "better-sqlite3"], + })); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should not create migrations columns with precision", () => { + Promise.all(connections.map(async (connection) => { + const sqlInMemory = await connection.driver.createSchemaBuilder().log(); + expect(sqlInMemory.upQueries).to.eql([]); + expect(sqlInMemory.downQueries).to.eql([]); + } + )) + }); + +}); From a1f3ef4569709cb893da26b3a01d956895af0a61 Mon Sep 17 00:00:00 2001 From: Umed Khudoiberdiev Date: Thu, 3 Sep 2020 12:49:30 +0500 Subject: [PATCH 2/3] awaited the test --- test/github-issues/6636/issue-6636.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/github-issues/6636/issue-6636.ts b/test/github-issues/6636/issue-6636.ts index bf40f67275..cad412b0c2 100644 --- a/test/github-issues/6636/issue-6636.ts +++ b/test/github-issues/6636/issue-6636.ts @@ -14,7 +14,7 @@ describe("github issues > #6636 migration issues with scale & precision", () => after(() => closeTestingConnections(connections)); it("should not create migrations columns with precision", () => { - Promise.all(connections.map(async (connection) => { + await Promise.all(connections.map(async (connection) => { const sqlInMemory = await connection.driver.createSchemaBuilder().log(); expect(sqlInMemory.upQueries).to.eql([]); expect(sqlInMemory.downQueries).to.eql([]); From 1a0d0932de7e8f7dc5004ba2fee824623f55ab40 Mon Sep 17 00:00:00 2001 From: Umed Khudoiberdiev Date: Thu, 3 Sep 2020 13:52:13 +0500 Subject: [PATCH 3/3] fix missing async --- test/github-issues/6636/issue-6636.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/github-issues/6636/issue-6636.ts b/test/github-issues/6636/issue-6636.ts index cad412b0c2..841f7c4a5a 100644 --- a/test/github-issues/6636/issue-6636.ts +++ b/test/github-issues/6636/issue-6636.ts @@ -13,7 +13,7 @@ describe("github issues > #6636 migration issues with scale & precision", () => beforeEach(() => reloadTestingDatabases(connections)); after(() => closeTestingConnections(connections)); - it("should not create migrations columns with precision", () => { + it("should not create migrations columns with precision", async () => { await Promise.all(connections.map(async (connection) => { const sqlInMemory = await connection.driver.createSchemaBuilder().log(); expect(sqlInMemory.upQueries).to.eql([]);