Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: get correct insert ids for multiple entities inserted #6668

Merged
merged 4 commits into from Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/driver/Driver.ts
Expand Up @@ -175,7 +175,7 @@ export interface Driver {
/**
* Creates generated map of values generated or returned by database after INSERT query.
*/
createGeneratedMap(metadata: EntityMetadata, insertResult: any): ObjectLiteral|undefined;
createGeneratedMap(metadata: EntityMetadata, insertResult: any, entityIndex?: number, entityNum?: number): ObjectLiteral|undefined;

/**
* Differentiate columns of this table and columns from the given column metadatas columns
Expand Down
6 changes: 4 additions & 2 deletions src/driver/mysql/MysqlDriver.ts
Expand Up @@ -709,11 +709,13 @@ export class MysqlDriver implements Driver {
/**
* Creates generated map of values generated or returned by database after INSERT query.
*/
createGeneratedMap(metadata: EntityMetadata, insertResult: any) {
createGeneratedMap(metadata: EntityMetadata, insertResult: any, entityIndex: number) {
const generatedMap = metadata.generatedColumns.reduce((map, generatedColumn) => {
let value: any;
if (generatedColumn.generationStrategy === "increment" && insertResult.insertId) {
value = insertResult.insertId;
// NOTE: When multiple rows is inserted by a single INSERT statement,
// `insertId` is the value generated for the first inserted row only.
value = insertResult.insertId + entityIndex;
// } else if (generatedColumn.generationStrategy === "uuid") {
// console.log("getting db value:", generatedColumn.databaseName);
// value = generatedColumn.getEntityValue(uuidMap);
Expand Down
6 changes: 4 additions & 2 deletions src/driver/sqlite-abstract/AbstractSqliteDriver.ts
Expand Up @@ -507,11 +507,13 @@ export abstract class AbstractSqliteDriver implements Driver {
/**
* Creates generated map of values generated or returned by database after INSERT query.
*/
createGeneratedMap(metadata: EntityMetadata, insertResult: any) {
createGeneratedMap(metadata: EntityMetadata, insertResult: any, entityIndex: number, entityNum: number) {
const generatedMap = metadata.generatedColumns.reduce((map, generatedColumn) => {
let value: any;
if (generatedColumn.generationStrategy === "increment" && insertResult) {
value = insertResult;
// NOTE: When INSERT statement is successfully completed, the last inserted row ID is returned.
// see also: SqliteQueryRunner.query()
value = insertResult - entityNum + entityIndex + 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have the same description like in mysql driver, so we can understand what INSERT statement returns and why we need to deduct entityNum and add entityIndex + 1 ?

From what I understood in case of sqlite insertResult is an id number for the last inserted row returned by sqlite?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pleerock Yes. see here
I added a note.

// } else if (generatedColumn.generationStrategy === "uuid") {
// value = insertValue[generatedColumn.databaseName];
}
Expand Down
2 changes: 1 addition & 1 deletion src/query-builder/ReturningResultsEntityUpdator.ts
Expand Up @@ -93,7 +93,7 @@ export class ReturningResultsEntityUpdator {
}
// get all values generated by a database for us
const result = Array.isArray(insertResult.raw) ? insertResult.raw[entityIndex] : insertResult.raw;
const generatedMap = this.queryRunner.connection.driver.createGeneratedMap(metadata, result) || {};
const generatedMap = this.queryRunner.connection.driver.createGeneratedMap(metadata, result, entityIndex, entities.length) || {};

// if database does not support uuid generation we need to get uuid values
// generated by orm and set them to the generatedMap
Expand Down
12 changes: 12 additions & 0 deletions test/github-issues/2131/entity/Post.ts
@@ -0,0 +1,12 @@
import { PrimaryGeneratedColumn, Entity, Column } from "../../../../src";

@Entity()
export class Post {

@PrimaryGeneratedColumn()
id: number | null;

@Column()
title: string;

}
53 changes: 53 additions & 0 deletions test/github-issues/2131/issue-2131.ts
@@ -0,0 +1,53 @@
import "reflect-metadata";
import {
closeTestingConnections,
createTestingConnections,
reloadTestingDatabases,
} from "../../utils/test-utils";
import { Connection } from "../../../src/connection/Connection";
import { expect } from "chai";
import { Post } from "./entity/Post";

describe("github issues > #2131 InsertResult return the same primary key", () => {
let connections: Connection[];
const posts: Post[] = [{
id: null,
title: "Post 1",
}, {
id: null,
title: "Post 2",
}, {
id: null,
title: "Post 3",
}, {
id: null,
title: "Post 4",
}];

before(
async () =>
(connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
enabledDrivers: ["sqlite", "mysql"],
}))
);
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should get correct insert ids for multiple entities inserted", () =>
Promise.all(
connections.map(async (connection) => {
await connection
.createQueryBuilder()
.insert()
.into(Post)
.values(posts)
.execute();

expect(posts[0].id).to.equal(1);
expect(posts[1].id).to.equal(2);
expect(posts[2].id).to.equal(3);
expect(posts[3].id).to.equal(4);
})
));
});