fix: multiple assignments to same column on UPDATE #2651 (#5598)

Author: pleerock

src/query-builder/UpdateQueryBuilder.ts

@@ -1,5 +1,6 @@
 import {CockroachDriver} from "../driver/cockroachdb/CockroachDriver";
 import {SapDriver} from "../driver/sap/SapDriver";
+import { ColumnMetadata } from "../metadata/ColumnMetadata";
 import {QueryBuilder} from "./QueryBuilder";
 import {ObjectLiteral} from "../common/ObjectLiteral";
 import {Connection} from "../connection/Connection";
@@ -381,6 +382,7 @@ export class UpdateQueryBuilder<Entity> extends QueryBuilder<Entity> implements
 
         // prepare columns and values to be updated
         const updateColumnAndValues: string[] = [];
+        const updatedColumns: ColumnMetadata[] = [];
         const newParameters: ObjectLiteral = {};
         let parametersCount =   this.connection.driver instanceof MysqlDriver ||
                                 this.connection.driver instanceof AuroraDataApiDriver ||
@@ -399,6 +401,7 @@ export class UpdateQueryBuilder<Entity> extends QueryBuilder<Entity> implements
 
                 columns.forEach(column => {
                     if (!column.isUpdate) { return; }
+                    updatedColumns.push(column);
 
                     const paramName = "upd_" + column.databaseName;
 
@@ -458,9 +461,9 @@ export class UpdateQueryBuilder<Entity> extends QueryBuilder<Entity> implements
                 });
             });
 
-            if (metadata.versionColumn)
+            if (metadata.versionColumn && updatedColumns.indexOf(metadata.versionColumn) === -1)
                 updateColumnAndValues.push(this.escape(metadata.versionColumn.databaseName) + " = " + this.escape(metadata.versionColumn.databaseName) + " + 1");
-            if (metadata.updateDateColumn)
+            if (metadata.updateDateColumn && updatedColumns.indexOf(metadata.updateDateColumn) === -1)
                 updateColumnAndValues.push(this.escape(metadata.updateDateColumn.databaseName) + " = CURRENT_TIMESTAMP"); // todo: fix issue with CURRENT_TIMESTAMP(6) being used, can "DEFAULT" be used?!
 
         } else {

test/github-issues/2651/entity/Post.ts

@@ -0,0 +1,17 @@
+import { UpdateDateColumn } from "../../../../src";
+import {Entity} from "../../../../src/decorator/entity/Entity";
+import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+import {Column} from "../../../../src/decorator/columns/Column";
+
+@Entity()
+export class Post {
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    title: string;
+
+    @UpdateDateColumn({ type: "timestamptz" })
+    updatedAt: Date;
+
+}

test/github-issues/2651/issue-2651.ts

@@ -0,0 +1,36 @@
+import "reflect-metadata";
+import { expect } from "chai";
+import {createTestingConnections, closeTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
+import {Connection} from "../../../src/connection/Connection";
+import {Post} from "./entity/Post";
+
+describe.only("github issues > #2651 set shouldn't have update statements twice when UpdateDate is in use", () => {
+
+    let connections: Connection[];
+    before(async () => connections = await createTestingConnections({
+        entities: [__dirname + "/entity/*{.js,.ts}"],
+        enabledDrivers: ["postgres"]
+    }));
+    beforeEach(() => reloadTestingDatabases(connections));
+    after(() => closeTestingConnections(connections));
+
+    it.only("should add and remove relations of an entity if given a mix of ids and objects", () => Promise.all(connections.map(async connection => {
+
+        const post1 = new Post();
+        post1.title = "post #1";
+        await connection.manager.save(post1);
+
+        // within this issue update was failing
+        await connection.manager.update(Post, {
+            id: 1
+        }, {
+            title: "updated post",
+            updatedAt: new Date()
+        });
+
+        const loadedPost1 = await connection.manager.findOneOrFail(Post, { id: 1 });
+        expect(loadedPost1.title).to.be.eql("updated post");
+
+    })));
+
+});