fix: datetime functions in column "default" leads to unnecessary quer…

…ies during synchronization (#7517) * fix #3991 * fix #3991 * improved test for #2737; fixed remaining issues in #2737; * fix #6412 * added test for #4281 and #4658 * fixed `NOW()` function usage in MySQL/MariaDB; fixed precision in datetime functions for MySQL/MariaDB; improved #3991 test for MySQL/MariaDB; * added test for #2333 * added test for #7381 * added "remove" operation check in #4658 test * fixed precision in datetime functions for PostgreSQl; improved #3991 test for PostgreSQl; * improved #3991 test for CockroachDB; * improved #3991 test for Sqlite; * improved #3991 test for Oracle and SQLServer;
typeorm · Apr 1, 2021 · 03f3285 · 03f3285
1 parent 0595d23
commit 03f3285
Show file tree

Hide file tree

Showing 25 changed files with 1,040 additions and 52 deletions.
diff --git a/src/driver/cockroachdb/CockroachDriver.ts b/src/driver/cockroachdb/CockroachDriver.ts
@@ -490,7 +490,13 @@ export class CockroachDriver implements Driver {
             return defaultValue === true ? "true" : "false";
 
         } else if (typeof defaultValue === "function") {
-            return defaultValue();
+            const value = defaultValue();
+            if (value.toUpperCase() === "CURRENT_TIMESTAMP") {
+                return "current_timestamp()";
+            } else if (value.toUpperCase() === "CURRENT_DATE") {
+                return "current_date()";
+            }
+            return value;
 
         } else if (typeof defaultValue === "string") {
             return `'${defaultValue}'${arrayCast}`;

diff --git a/src/driver/cockroachdb/CockroachQueryRunner.ts b/src/driver/cockroachdb/CockroachQueryRunner.ts
@@ -1527,7 +1527,7 @@ export class CockroachQueryRunner extends BaseQueryRunner implements QueryRunner
                             tableColumn.isGenerated = true;
                             tableColumn.generationStrategy = "uuid";
 
-                        } else {
+                        } else  {
                             tableColumn.default = dbColumn["column_default"].replace(/:::.*/, "");
                             tableColumn.default = tableColumn.default.replace(/^(-?[\d\.]+)$/, "($1)");
                         }

diff --git a/src/driver/mysql/MysqlDriver.ts b/src/driver/mysql/MysqlDriver.ts
@@ -602,8 +602,8 @@ export class MysqlDriver implements Driver {
             return defaultValue === true ? "1" : "0";
 
         } else if (typeof defaultValue === "function") {
-            return defaultValue();
-
+            const value = defaultValue();
+            return this.normalizeDatetimeFunction(value)
         } else {
             return defaultValue;
         }
@@ -737,35 +737,12 @@ export class MysqlDriver implements Driver {
             if (!tableColumn)
                 return false; // we don't need new columns, we only need exist and changed
 
-            // console.log("table:", columnMetadata.entityMetadata.tableName);
-            // console.log("name:", tableColumn.name, columnMetadata.databaseName);
-            // console.log("type:", tableColumn.type, this.normalizeType(columnMetadata));
-            // console.log("length:", tableColumn.length, columnMetadata.length);
-            // console.log("width:", tableColumn.width, columnMetadata.width);
-            // console.log("precision:", tableColumn.precision, columnMetadata.precision);
-            // console.log("scale:", tableColumn.scale, columnMetadata.scale);
-            // console.log("zerofill:", tableColumn.zerofill, columnMetadata.zerofill);
-            // console.log("unsigned:", tableColumn.unsigned, columnMetadata.unsigned);
-            // console.log("asExpression:", tableColumn.asExpression, columnMetadata.asExpression);
-            // console.log("generatedType:", tableColumn.generatedType, columnMetadata.generatedType);
-            // console.log("comment:", tableColumn.comment, columnMetadata.comment);
-            // console.log("default:", tableColumn.default, columnMetadata.default);
-            // console.log("enum:", tableColumn.enum, columnMetadata.enum);
-            // console.log("default changed:", !this.compareDefaultValues(this.normalizeDefault(columnMetadata), tableColumn.default));
-            // console.log("onUpdate:", tableColumn.onUpdate, columnMetadata.onUpdate);
-            // console.log("isPrimary:", tableColumn.isPrimary, columnMetadata.isPrimary);
-            // console.log("isNullable:", tableColumn.isNullable, columnMetadata.isNullable);
-            // console.log("isUnique:", tableColumn.isUnique, this.normalizeIsUnique(columnMetadata));
-            // console.log("isGenerated:", tableColumn.isGenerated, columnMetadata.isGenerated);
-            // console.log((columnMetadata.generationStrategy !== "uuid" && tableColumn.isGenerated !== columnMetadata.isGenerated));
-            // console.log("==========================================");
-
             let columnMetadataLength = columnMetadata.length;
             if (!columnMetadataLength && columnMetadata.generationStrategy === "uuid") { // fixing #3374
                 columnMetadataLength = this.getColumnLength(columnMetadata);
             }
 
-            return tableColumn.name !== columnMetadata.databaseName
+            const isColumnChanged = tableColumn.name !== columnMetadata.databaseName
                 || tableColumn.type !== this.normalizeType(columnMetadata)
                 || tableColumn.length !== columnMetadataLength
                 || tableColumn.width !== columnMetadata.width
@@ -778,11 +755,39 @@ export class MysqlDriver implements Driver {
                 || tableColumn.comment !== columnMetadata.comment
                 || !this.compareDefaultValues(this.normalizeDefault(columnMetadata), tableColumn.default)
                 || (tableColumn.enum && columnMetadata.enum && !OrmUtils.isArraysEqual(tableColumn.enum, columnMetadata.enum.map(val => val + "")))
-                || tableColumn.onUpdate !== columnMetadata.onUpdate
+                || tableColumn.onUpdate !== this.normalizeDatetimeFunction(columnMetadata.onUpdate)
                 || tableColumn.isPrimary !== columnMetadata.isPrimary
                 || tableColumn.isNullable !== columnMetadata.isNullable
                 || tableColumn.isUnique !== this.normalizeIsUnique(columnMetadata)
                 || (columnMetadata.generationStrategy !== "uuid" && tableColumn.isGenerated !== columnMetadata.isGenerated);
+
+            // DEBUG SECTION
+            // if (isColumnChanged) {
+            //     console.log("table:", columnMetadata.entityMetadata.tableName);
+            //     console.log("name:", tableColumn.name, columnMetadata.databaseName);
+            //     console.log("type:", tableColumn.type, this.normalizeType(columnMetadata));
+            //     console.log("length:", tableColumn.length, columnMetadata.length);
+            //     console.log("width:", tableColumn.width, columnMetadata.width);
+            //     console.log("precision:", tableColumn.precision, columnMetadata.precision);
+            //     console.log("scale:", tableColumn.scale, columnMetadata.scale);
+            //     console.log("zerofill:", tableColumn.zerofill, columnMetadata.zerofill);
+            //     console.log("unsigned:", tableColumn.unsigned, columnMetadata.unsigned);
+            //     console.log("asExpression:", tableColumn.asExpression, columnMetadata.asExpression);
+            //     console.log("generatedType:", tableColumn.generatedType, columnMetadata.generatedType);
+            //     console.log("comment:", tableColumn.comment, columnMetadata.comment);
+            //     console.log("default:", tableColumn.default, this.normalizeDefault(columnMetadata));
+            //     console.log("enum:", tableColumn.enum, columnMetadata.enum);
+            //     console.log("default changed:", !this.compareDefaultValues(this.normalizeDefault(columnMetadata), tableColumn.default));
+            //     console.log("onUpdate:", tableColumn.onUpdate, this.normalizeOnUpdate(columnMetadata.onUpdate));
+            //     console.log("isPrimary:", tableColumn.isPrimary, columnMetadata.isPrimary);
+            //     console.log("isNullable:", tableColumn.isNullable, columnMetadata.isNullable);
+            //     console.log("isUnique:", tableColumn.isUnique, this.normalizeIsUnique(columnMetadata));
+            //     console.log("isGenerated:", tableColumn.isGenerated, columnMetadata.isGenerated);
+            //     console.log((columnMetadata.generationStrategy !== "uuid" && tableColumn.isGenerated !== columnMetadata.isGenerated));
+            //     console.log("==========================================");
+            // }
+
+            return isColumnChanged
         });
     }
 
@@ -930,4 +935,28 @@ export class MysqlDriver implements Driver {
         return columnMetadataValue === databaseValue;
     }
 
+    /**
+     * If parameter is a datetime function, e.g. "CURRENT_TIMESTAMP", normalizes it.
+     * Otherwise returns original input.
+     */
+    protected normalizeDatetimeFunction(value?: string) {
+        if (!value) return value
+
+        // check if input is datetime function
+        const isDatetimeFunction = value.toUpperCase().indexOf("CURRENT_TIMESTAMP") !== -1
+            || value.toUpperCase().indexOf("NOW") !== -1;
+
+        if (isDatetimeFunction) {
+            // extract precision, e.g. "(3)"
+            const precision = value.match(/\(\d+\)/)
+            if (this.options.type === "mariadb") {
+                return precision ? `CURRENT_TIMESTAMP${precision[0]}` : "CURRENT_TIMESTAMP()";
+            } else {
+                return precision ? `CURRENT_TIMESTAMP${precision[0]}` : "CURRENT_TIMESTAMP";
+            }
+        } else {
+            return value
+        }
+    }
+
 }
diff --git a/src/driver/postgres/PostgresDriver.ts b/src/driver/postgres/PostgresDriver.ts
@@ -750,7 +750,8 @@ export class PostgresDriver implements Driver {
             return defaultValue === true ? "true" : "false";
 
         } else if (typeof defaultValue === "function") {
-            return defaultValue();
+            const value = defaultValue();
+            return this.normalizeDatetimeFunction(value)
 
         } else if (typeof defaultValue === "object") {
             return `'${JSON.stringify(defaultValue)}'`;
@@ -903,7 +904,6 @@ export class PostgresDriver implements Driver {
             //     console.log("comment:", tableColumn.comment, columnMetadata.comment);
             //     console.log("enumName:", tableColumn.enumName, columnMetadata.enumName);
             //     console.log("enum:", tableColumn.enum && columnMetadata.enum && !OrmUtils.isArraysEqual(tableColumn.enum, columnMetadata.enum.map(val => val + "")));
-            //     console.log("onUpdate:", tableColumn.onUpdate, columnMetadata.onUpdate);
             //     console.log("isPrimary:", tableColumn.isPrimary, columnMetadata.isPrimary);
             //     console.log("isNullable:", tableColumn.isNullable, columnMetadata.isNullable);
             //     console.log("isUnique:", tableColumn.isUnique, this.normalizeIsUnique(columnMetadata));
@@ -1069,4 +1069,41 @@ export class PostgresDriver implements Driver {
         });
     }
 
+    /**
+     * If parameter is a datetime function, e.g. "CURRENT_TIMESTAMP", normalizes it.
+     * Otherwise returns original input.
+     */
+    protected normalizeDatetimeFunction(value: string) {
+        // check if input is datetime function
+        const upperCaseValue = value.toUpperCase()
+        const isDatetimeFunction = upperCaseValue.indexOf("CURRENT_TIMESTAMP") !== -1
+            || upperCaseValue.indexOf("CURRENT_DATE") !== -1
+            || upperCaseValue.indexOf("CURRENT_TIME") !== -1
+            || upperCaseValue.indexOf("LOCALTIMESTAMP") !== -1
+            || upperCaseValue.indexOf("LOCALTIME") !== -1;
+
+        if (isDatetimeFunction) {
+            // extract precision, e.g. "(3)"
+            const precision = value.match(/\(\d+\)/)
+
+            if (upperCaseValue.indexOf("CURRENT_TIMESTAMP") !== -1) {
+                return precision ? `('now'::text)::timestamp${precision[0]} with time zone` : "now()";
+
+            } else if (upperCaseValue === "CURRENT_DATE") {
+                return "('now'::text)::date"
+
+            } else if (upperCaseValue.indexOf("CURRENT_TIME") !== -1) {
+                return precision ? `('now'::text)::time${precision[0]} with time zone` : "('now'::text)::time with time zone"
+
+            } else if (upperCaseValue.indexOf("LOCALTIMESTAMP") !== -1) {
+                return precision ? `('now'::text)::timestamp${precision[0]} without time zone` : "('now'::text)::timestamp without time zone"
+
+            } else if (upperCaseValue.indexOf("LOCALTIME") !== -1) {
+                return precision ? `('now'::text)::time${precision[0]} without time zone` : "('now'::text)::time without time zone"
+            }
+        }
+
+        return value
+    }
+
 }
diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts
@@ -1679,6 +1679,8 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner
                         } else if (dbColumn["column_default"] === "gen_random_uuid()" || /^uuid_generate_v\d\(\)/.test(dbColumn["column_default"])) {
                             tableColumn.isGenerated = true;
                             tableColumn.generationStrategy = "uuid";
+                        } else if (dbColumn["column_default"] === "now()" || dbColumn["column_default"].indexOf("'now'::text") !== -1) {
+                            tableColumn.default = dbColumn["column_default"]
                         } else {
                             tableColumn.default = dbColumn["column_default"].replace(/::.*/, "");
                             tableColumn.default = tableColumn.default.replace(/^(-?\d+)$/, "'$1'");

diff --git a/src/driver/sqlserver/SqlServerDriver.ts b/src/driver/sqlserver/SqlServerDriver.ts
@@ -501,7 +501,11 @@ export class SqlServerDriver implements Driver {
             return defaultValue === true ? "1" : "0";
 
         } else if (typeof defaultValue === "function") {
-            return /*"(" + */defaultValue()/* + ")"*/;
+            const value = defaultValue();
+            if (value.toUpperCase() === "CURRENT_TIMESTAMP") {
+                return "getdate()"
+            }
+            return value
 
         } else if (typeof defaultValue === "string") {
             return `'${defaultValue}'`;

diff --git a/src/metadata/ColumnMetadata.ts b/src/metadata/ColumnMetadata.ts
@@ -368,7 +368,7 @@ export class ColumnMetadata {
         }
         if (options.args.options.unsigned)
             this.unsigned = options.args.options.unsigned;
-        if (options.args.options.precision !== undefined)
+        if (options.args.options.precision !== null)
             this.precision = options.args.options.precision;
         if (options.args.options.enum) {
             if (options.args.options.enum instanceof Object && !Array.isArray(options.args.options.enum)) {
@@ -413,7 +413,8 @@ export class ColumnMetadata {
                 this.type = options.connection.driver.mappedDataTypes.createDate;
             if (!this.default)
                 this.default = () => options.connection.driver.mappedDataTypes.createDateDefault;
-            if (this.precision === undefined && options.connection.driver.mappedDataTypes.createDatePrecision)
+            // skip precision if it was explicitly set to "null" in column options. Otherwise use default precision if it exist.
+            if (this.precision === undefined && options.args.options.precision === undefined && options.connection.driver.mappedDataTypes.createDatePrecision)
                 this.precision = options.connection.driver.mappedDataTypes.createDatePrecision;
         }
         if (this.isUpdateDate) {
@@ -423,15 +424,17 @@ export class ColumnMetadata {
                 this.default = () => options.connection.driver.mappedDataTypes.updateDateDefault;
             if (!this.onUpdate)
                 this.onUpdate = options.connection.driver.mappedDataTypes.updateDateDefault;
-            if (this.precision === undefined && options.connection.driver.mappedDataTypes.updateDatePrecision)
+            // skip precision if it was explicitly set to "null" in column options. Otherwise use default precision if it exist.
+            if (this.precision === undefined && options.args.options.precision === undefined && options.connection.driver.mappedDataTypes.updateDatePrecision)
                 this.precision = options.connection.driver.mappedDataTypes.updateDatePrecision;
         }
         if (this.isDeleteDate) {
             if (!this.type)
                 this.type = options.connection.driver.mappedDataTypes.deleteDate;
             if (!this.isNullable)
                 this.isNullable = options.connection.driver.mappedDataTypes.deleteDateNullable;
-            if (this.precision === undefined && options.connection.driver.mappedDataTypes.deleteDatePrecision)
+            // skip precision if it was explicitly set to "null" in column options. Otherwise use default precision if it exist.
+            if (this.precision === undefined && options.args.options.precision === undefined && options.connection.driver.mappedDataTypes.deleteDatePrecision)
                 this.precision = options.connection.driver.mappedDataTypes.deleteDatePrecision;
         }
         if (this.isVersion)

diff --git a/test/functional/query-runner/rename-column.ts b/test/functional/query-runner/rename-column.ts
@@ -24,10 +24,6 @@ describe("query runner > rename column", () => {
 
     it("should correctly rename column and revert rename", () => Promise.all(connections.map(async connection => {
 
-        // TODO: https://github.com/cockroachdb/cockroach/issues/32555
-        if (connection.driver instanceof CockroachDriver)
-            return;
-
         const queryRunner = connection.createQueryRunner();
 
         let table = await queryRunner.getTable("post");
@@ -58,10 +54,6 @@ describe("query runner > rename column", () => {
 
     it("should correctly rename column with all constraints and revert rename", () => Promise.all(connections.map(async connection => {
 
-        // TODO: https://github.com/cockroachdb/cockroach/issues/32555
-        if (connection.driver instanceof CockroachDriver)
-            return;
-
         const queryRunner = connection.createQueryRunner();
 
         let table = await queryRunner.getTable("post");
@@ -106,10 +98,6 @@ describe("query runner > rename column", () => {
 
     it("should correctly rename column with all constraints in custom table schema and database and revert rename", () => Promise.all(connections.map(async connection => {
 
-        // TODO: https://github.com/cockroachdb/cockroach/issues/32555
-        if (connection.driver instanceof CockroachDriver)
-            return;
-
         const queryRunner = connection.createQueryRunner();
         let table: Table|undefined;
 

diff --git a/test/github-issues/2333/entity/Test.ts b/test/github-issues/2333/entity/Test.ts
@@ -0,0 +1,12 @@
+import {Column, Entity, PrimaryGeneratedColumn} from "../../../../src";
+
+@Entity("test")
+export class Test {
+
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column({ type: "datetime", nullable: true, default: null })
+    publish_date: Date;
+
+}
diff --git a/test/github-issues/2333/issue-2333.ts b/test/github-issues/2333/issue-2333.ts
@@ -0,0 +1,27 @@
+import {Connection} from "../../../src";
+import {closeTestingConnections, createTestingConnections} from "../../utils/test-utils";
+import {Test} from "./entity/Test";
+
+describe("github issues > #2333 datetime column showing changed on every schema:sync run", () => {
+    let connections: Connection[];
+    before(async () => connections = await createTestingConnections({
+        enabledDrivers: ["mysql", "mariadb"],
+        schemaCreate: false,
+        dropSchema: true,
+        entities: [Test],
+    }));
+    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);
+    })));
+})
diff --git a/test/github-issues/2737/entity/TestEntity.ts b/test/github-issues/2737/entity/TestEntity.ts
@@ -1,6 +1,8 @@
-import { Entity } from "../../../../src/decorator/entity/Entity";
-import { Column } from "../../../../src/decorator/columns/Column";
-import { PrimaryGeneratedColumn } from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+import { Entity } from "../../../../src";
+import { Column } from "../../../../src";
+import { PrimaryGeneratedColumn } from "../../../../src";
+import {CreateDateColumn} from "../../../../src";
+
 @Entity()
 export class TestEntity {
     @PrimaryGeneratedColumn()
@@ -11,4 +13,16 @@ export class TestEntity {
 
     @Column({ type: "varchar", length: 100, nullable: true, unique: false })
     nonunique_column: string;
+
+    @CreateDateColumn({ type: "timestamp", default: () => "CURRENT_TIMESTAMP(6)" })
+    from: Date;
+
+    @CreateDateColumn({ precision: 3, type: "timestamp", default: () => "CURRENT_TIMESTAMP(3)" })
+    from2: Date;
+
+    @CreateDateColumn({ precision: null, type: "timestamp", default: () => "CURRENT_TIMESTAMP" })
+    from3: Date;
+
+    @Column({ precision: null, type: "timestamp", default: null, nullable: true })
+    to: Date;
 }