From 4fc4a1b4eb4e7d54d52e8559b9f04a94cd3ad154 Mon Sep 17 00:00:00 2001 From: Json Choi <1890mah@gmail.com> Date: Fri, 4 Sep 2020 19:56:46 +0900 Subject: [PATCH] fix: make only a single SELECT to get inserted default and generated values of multiple entities (#6669) * fix: re-select inserted default and generated values with a single SELECT closes #6266 * test: add test case for fix of #6266 * fix lint error --- .../ReturningResultsEntityUpdator.ts | 41 +++++++------ test/github-issues/6266/entity/Post.ts | 14 +++++ test/github-issues/6266/issue-6266.ts | 60 +++++++++++++++++++ 3 files changed, 96 insertions(+), 19 deletions(-) create mode 100644 test/github-issues/6266/entity/Post.ts create mode 100644 test/github-issues/6266/issue-6266.ts diff --git a/src/query-builder/ReturningResultsEntityUpdator.ts b/src/query-builder/ReturningResultsEntityUpdator.ts index c12e4794b5..6e9b41207a 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) => { 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(); + }) + )); +});