Skip to content

Commit

Permalink
fix: support multiple row insert on oracle
Browse files Browse the repository at this point in the history
change generated insert query to allow inserting an array of objects to oracle

fixes: #2434
  • Loading branch information
wfpena committed Oct 17, 2020
1 parent dee1758 commit 1c3db34
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 8 deletions.
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()
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;
}
91 changes: 91 additions & 0 deletions test/github-issues/2434/issue-2434.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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([
{id: 1, itemName: "item name 1"},
{id: 2, itemName: "item name 2"},
])
.execute();
expect(result.raw).to.be.equal(2);
expect(result.identifiers).to.deep.equal([
{id: 1},
{id: 2},
]);

})));

it("should still insert one row with QueryBuilder", () => Promise.all(connections.map(async connection => {
const result = await connection.createQueryBuilder()
.insert()
.into(Item)
.values({id: 20, itemName: "item name 20"})
.execute();
expect(result.identifiers).to.deep.equal([{id: 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});
})));

});

0 comments on commit 1c3db34

Please sign in to comment.