From 688e0700df077b5af980f0097cd859536774e613 Mon Sep 17 00:00:00 2001 From: Jiwon Choi <1890mah@gmail.com> Date: Fri, 4 Sep 2020 17:51:55 +0900 Subject: [PATCH 1/3] fix: re-select inserted default and generated values with a single SELECT closes #6266 --- .../ReturningResultsEntityUpdator.ts | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/query-builder/ReturningResultsEntityUpdator.ts b/src/query-builder/ReturningResultsEntityUpdator.ts index 7dcd9f100d..f29ba33f89 100644 --- a/src/query-builder/ReturningResultsEntityUpdator.ts +++ b/src/query-builder/ReturningResultsEntityUpdator.ts @@ -117,26 +117,29 @@ export class ReturningResultsEntityUpdator { // for postgres and mssql we use returning/output statement to get values of inserted default and generated values // for other drivers we have to re-select this data from the database if (this.queryRunner.connection.driver.isReturningSqlSupported() === false && insertionColumns.length > 0) { - await Promise.all(entities.map(async (entity, entityIndex) => { + const entityIds = entities.map((entity) => { const entityId = metadata.getEntityIdMap(entity)!; - - // to select just inserted entity we need a criteria to select by. - // for newly inserted entities in drivers which do not support returning statement - // row identifier can only be an increment column - // (since its the only thing that can be generated by those databases) - // or (and) other primary key which is defined by a user and inserted value has it - - const returningResult: any = await this.queryRunner.manager - .createQueryBuilder() - .select(metadata.primaryColumns.map(column => metadata.targetName + "." + column.propertyPath)) - .addSelect(insertionColumns.map(column => metadata.targetName + "." + column.propertyPath)) - .from(metadata.target, metadata.targetName) - .where(entityId) - .setOption("create-pojo") // use POJO because created object can contain default values, e.g. property = null and those properties maight be overridden by merge process - .getOne(); - - this.queryRunner.manager.merge(metadata.target as any, generatedMaps[entityIndex], returningResult); - })); + return entityId; + }); + + // to select just inserted entities we need a criteria to select by. + // for newly inserted entities in drivers which do not support returning statement + // row identifier can only be an increment column + // (since its the only thing that can be generated by those databases) + // or (and) other primary key which is defined by a user and inserted value has it + + const returningResult: any = await this.queryRunner.manager + .createQueryBuilder() + .select(metadata.primaryColumns.map(column => metadata.targetName + "." + column.propertyPath)) + .addSelect(insertionColumns.map(column => metadata.targetName + "." + column.propertyPath)) + .from(metadata.target, metadata.targetName) + .where(entityIds) + .setOption("create-pojo") // use POJO because created object can contain default values, e.g. property = null and those properties maight be overridden by merge process + .getMany(); + + entities.forEach((entity, entityIndex) => { + this.queryRunner.manager.merge(metadata.target as any, generatedMaps[entityIndex], returningResult[entityIndex]); + }) } entities.forEach((entity, entityIndex) => { From 34d279004e55a43a1597162366513839f3b82853 Mon Sep 17 00:00:00 2001 From: Jiwon Choi <1890mah@gmail.com> Date: Fri, 4 Sep 2020 18:04:44 +0900 Subject: [PATCH 2/3] test: add test case for fix of #6266 --- test/github-issues/6266/entity/Post.ts | 14 ++++++ test/github-issues/6266/issue-6266.ts | 60 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 test/github-issues/6266/entity/Post.ts create mode 100644 test/github-issues/6266/issue-6266.ts diff --git a/test/github-issues/6266/entity/Post.ts b/test/github-issues/6266/entity/Post.ts new file mode 100644 index 0000000000..94e6f92ea5 --- /dev/null +++ b/test/github-issues/6266/entity/Post.ts @@ -0,0 +1,14 @@ +import { PrimaryGeneratedColumn, Entity, Column, CreateDateColumn } from "../../../../src"; + +@Entity() +export class Post { + + @PrimaryGeneratedColumn() + id?: number; + + @Column() + title: string; + + @CreateDateColumn() + readonly createdAt?: Date; +} diff --git a/test/github-issues/6266/issue-6266.ts b/test/github-issues/6266/issue-6266.ts new file mode 100644 index 0000000000..8e8a79e9c0 --- /dev/null +++ b/test/github-issues/6266/issue-6266.ts @@ -0,0 +1,60 @@ +import "reflect-metadata"; +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils"; +import { Connection } from "../../../src/connection/Connection"; +import { Post } from "./entity/Post"; +import sinon from "sinon"; +import { SelectQueryBuilder } from "../../../src"; +import { assert } from "chai"; + +describe("github issues > #6266 Many identical selects after insert bunch of items", () => { + let connections: Connection[]; + const posts: Post[] = [ + { + title: "Post 1", + }, + { + title: "Post 2", + }, + { + title: "Post 3", + }, + { + title: "Post 4", + }, + ]; + + before( + async () => + (connections = await createTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + enabledDrivers: ["mysql"], + })) + ); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should execute a single SELECT to get inserted default and generated values of multiple entities", () => + Promise.all( + connections.map(async (connection) => { + const selectSpy = sinon.spy( + SelectQueryBuilder.prototype, + "select" + ); + + await connection + .createQueryBuilder() + .insert() + .into(Post) + .values(posts) + .execute(); + + assert.strictEqual(selectSpy.calledOnce, true); + + selectSpy.restore(); + }) + )); +}); From 6401136b3177bcfbbad3c6fc9345894f7ed87d4b Mon Sep 17 00:00:00 2001 From: Jiwon Choi <1890mah@gmail.com> Date: Fri, 4 Sep 2020 18:41:12 +0900 Subject: [PATCH 3/3] fix lint error --- src/query-builder/ReturningResultsEntityUpdator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query-builder/ReturningResultsEntityUpdator.ts b/src/query-builder/ReturningResultsEntityUpdator.ts index f29ba33f89..1e50b57fa4 100644 --- a/src/query-builder/ReturningResultsEntityUpdator.ts +++ b/src/query-builder/ReturningResultsEntityUpdator.ts @@ -139,7 +139,7 @@ export class ReturningResultsEntityUpdator { entities.forEach((entity, entityIndex) => { this.queryRunner.manager.merge(metadata.target as any, generatedMaps[entityIndex], returningResult[entityIndex]); - }) + }); } entities.forEach((entity, entityIndex) => {