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: support multiple row insert on oracle #6927

Merged
merged 1 commit into from
Oct 26, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions src/query-builder/InsertQueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {
// console.time(".prepare returning statement");
const returningResultsEntityUpdator = new ReturningResultsEntityUpdator(queryRunner, this.expressionMap);
if (this.expressionMap.updateEntity === true && this.expressionMap.mainAlias!.hasMetadata) {
this.expressionMap.extraReturningColumns = returningResultsEntityUpdator.getInsertionReturningColumns();

if (!(valueSets.length > 1 && this.connection.driver instanceof OracleDriver)) {
this.expressionMap.extraReturningColumns = returningResultsEntityUpdator.getInsertionReturningColumns();
}
if (this.expressionMap.extraReturningColumns.length > 0 && this.connection.driver instanceof SqlServerDriver) {
declareSql = this.connection.driver.buildTableVariableDeclaration("@OutputTable", this.expressionMap.extraReturningColumns);
selectOutputSql = `SELECT * FROM @OutputTable`;
Expand Down Expand Up @@ -293,7 +294,7 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {
protected createInsertExpression() {
const tableName = this.getTableName(this.getMainTableName());
const valuesExpression = this.createValuesExpression(); // its important to get values before returning expression because oracle rely on native parameters and ordering of them is important
const returningExpression = this.createReturningExpression();
const returningExpression = (this.connection.driver instanceof OracleDriver && this.getValueSets().length > 1) ? null : this.createReturningExpression(); // oracle doesnt support returning with multi-row insert
const columnsExpression = this.createColumnNamesExpression();
let query = "INSERT ";

Expand All @@ -318,7 +319,11 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {

// add VALUES expression
if (valuesExpression) {
query += ` VALUES ${valuesExpression}`;
if (this.connection.driver instanceof OracleDriver && this.getValueSets().length > 1) {
query += ` ${valuesExpression}`;
} else {
query += ` VALUES ${valuesExpression}`;
}
} else {
if (this.connection.driver instanceof MysqlDriver || this.connection.driver instanceof AuroraDataApiDriver) { // special syntax for mysql DEFAULT VALUES insertion
query += " VALUES ()";
Expand Down Expand Up @@ -413,7 +418,11 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {
valueSets.forEach((valueSet, valueSetIndex) => {
columns.forEach((column, columnIndex) => {
if (columnIndex === 0) {
expression += "(";
if (this.connection.driver instanceof OracleDriver && valueSets.length > 1) {
expression += " SELECT ";
} else {
expression += "(";
}
}
const paramName = "i" + valueSetIndex + "_" + column.databaseName;

Expand Down Expand Up @@ -473,7 +482,7 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {

// if value for this column was not provided then insert default value
} else if (value === undefined) {
if (this.connection.driver instanceof AbstractSqliteDriver || this.connection.driver instanceof SapDriver) { // unfortunately sqlite does not support DEFAULT expression in INSERT queries
if ((this.connection.driver instanceof OracleDriver && valueSets.length > 1) || this.connection.driver instanceof AbstractSqliteDriver || this.connection.driver instanceof SapDriver) { // unfortunately sqlite does not support DEFAULT expression in INSERT queries
if (column.default !== undefined) { // try to use default defined in the column
expression += this.connection.driver.normalizeDefault(column);
} else {
Expand Down Expand Up @@ -522,9 +531,17 @@ export class InsertQueryBuilder<Entity> extends QueryBuilder<Entity> {

if (columnIndex === columns.length - 1) {
if (valueSetIndex === valueSets.length - 1) {
expression += ")";
if (this.connection.driver instanceof OracleDriver && valueSets.length > 1) {
expression += " FROM DUAL ";
} else {
expression += ")";
}
} else {
expression += "), ";
if (this.connection.driver instanceof OracleDriver && valueSets.length > 1) {
expression += " FROM DUAL UNION ALL ";
} else {
expression += "), ";
}
}
} else {
expression += ", ";
Expand Down
18 changes: 18 additions & 0 deletions test/github-issues/2434/entity/Item.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { CreateDateColumn, Column } from "../../../../src";
import { PrimaryGeneratedColumn } from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
import { Entity } from "../../../../src/decorator/entity/Entity";

@Entity("ITEM")
export class Item {
@PrimaryGeneratedColumn("uuid")
id: number;

@CreateDateColumn()
date: Date;

@Column()
itemName: string;

@Column({nullable: true})
itemDescription?: string;
}
16 changes: 16 additions & 0 deletions test/github-issues/2434/entity/Post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {Entity} from "../../../../src/decorator/entity/Entity";
import {PrimaryColumn} from "../../../../src/decorator/columns/PrimaryColumn";
import {Column} from "../../../../src/decorator/columns/Column";

@Entity({name: "POST"})
export class Post {

@PrimaryColumn()
id: number;

@Column({ nullable: true })
title?: string;

@Column({name: "named_column", nullable: true})
namedColumn?: string;
}
90 changes: 90 additions & 0 deletions test/github-issues/2434/issue-2434.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import "reflect-metadata";
import {Connection} from "../../../src";
import {Post} from './entity/Post';
import {Item} from './entity/Item';
import { closeTestingConnections, reloadTestingDatabases, createTestingConnections } from '../../utils/test-utils';
import { expect } from 'chai';

describe("github issues > #2434 QueryBuilder insert for Oracle failed", () => {
let connections: Connection[] = [];
before(async () => connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
enabledDrivers: ["oracle"]
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should insert multiple rows with QueryBuilder", () => Promise.all(connections.map(async connection => {
const result = await connection.createQueryBuilder()
.insert()
.into(Post)
.values([
{id: 5, title: "title 1"},
{id: 6},
])
.execute();
expect(result.raw).to.be.equal(2);
expect(result.identifiers).to.deep.equal([
{id: 5},
{id: 6},
]);
})));

it("should throw ORA-00001 error if constraint violated when inserting multiple rows", () => Promise.all(connections.map(async connection => {
try {
await connection.createQueryBuilder()
.insert()
.into(Post)
.values([
{id: 6, title: "title 3"},
{id: 6},
])
.execute();
} catch(err) {
expect(err.message).to.contain("ORA-00001");
}
})));

it("should insert multiple rows of entity with generated columns with QueryBuilder", () => Promise.all(connections.map(async connection => {
const result = await connection.createQueryBuilder()
.insert()
.into(Item)
.values([
{itemName: "item name 1"},
{itemName: "item name 2"},
])
.execute();
expect(result.raw).to.be.equal(2);
const items = await connection.getRepository(Item).find();
expect(items.length).to.be.equal(2);
})));

it("should still insert one row with QueryBuilder", () => Promise.all(connections.map(async connection => {
const result = await connection.createQueryBuilder()
.insert()
.into(Item)
.values({itemName: "item name 20"})
.execute();
expect(result.identifiers.length).to.be.equal(1);
const items = await connection.getRepository(Item).find();
expect(items[0].itemName).to.be.equal("item name 20");
})));

it("should still insert multiple rows with save", () => Promise.all(connections.map(async connection => {
const result = await connection.getRepository(Post).save([
{id: 8, namedColumn: "test col 1"},
{id: 9, title: "title id 9"},
]);
expect(result).to.deep.equal([
{id: 8, title: null, namedColumn: "test col 1"},
{id: 9, title: "title id 9", namedColumn: null},
]);

})));

it("should still insert one row with save", () => Promise.all(connections.map(async connection => {
const result = await connection.getRepository(Post).save({id: 10});
expect(result).to.deep.equal({id: 10, title: null, namedColumn: null});
})));

});