Skip to content
This repository has been archived by the owner on Mar 4, 2019. It is now read-only.

Commit

Permalink
fix: better messaging for deep insert errors (closes #556) (#571)
Browse files Browse the repository at this point in the history
  • Loading branch information
dmfay committed Mar 20, 2018
1 parent 4fb9584 commit 8ff4045
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 5 deletions.
7 changes: 6 additions & 1 deletion lib/database.js
Expand Up @@ -334,7 +334,12 @@ Database.prototype.query = function (query, params = [], options = {}) {
if (query instanceof this.pgp.QueryFile || _.isString(query)) {
sql = query;
} else {
sql = query.format();
try {
sql = query.format();
} catch (err) {
return this.$p.reject(err);
}

params = query.params;
options = query;
}
Expand Down
12 changes: 8 additions & 4 deletions lib/statement/insert.js
Expand Up @@ -41,10 +41,6 @@ const Insert = function (source, record, options = {}) {
const recordParams = prepareParams(this.columns, this.records);

if (this.deepInsert && this.junctions.length) {
if (this.records.length > 1) {
throw new Error('Deep insert is only supported for single records');
}

// append all junction params (that aren't stubbing out the foreign keys)
// to the insert's parameter list
// TODO generate junction field set to allow more flexibility between
Expand All @@ -66,6 +62,10 @@ const Insert = function (source, record, options = {}) {
* @return {String} A SQL INSERT statement.
*/
Insert.prototype.format = function () {
if (this.deepInsert && this.junctions.length && this.records.length > 1) {
throw new Error('Found potential deep insert definitions in the record array. Deep insert is only supported for single records. If you are not attempting a deep insert, ensure that your records do not contain non-column keys or use the {deepInsert: false} option.');
}

const quotedColumns = this.columns.map(f => `"${f}"`);

let sql = `INSERT INTO ${this.source.delimitedFullName} (${quotedColumns.join(', ')}) VALUES `;
Expand All @@ -91,6 +91,10 @@ Insert.prototype.format = function () {
const sourcePkList = `"${this.source.pk.join('", "')}"`;

const junctionQueries = _.reduce(this.junctions, (queries, j, idx) => {
if (!Array.isArray(this.records[0][j])) {
throw new Error('Attempted a deep insert with a bad junction definition. If you did not intend a deep insert, ensure that your record only contains values for database columns or disable this functionality with the {deepInsert: false} option.');
}

return queries.concat(this.records[0][j].map((r, jdx) => {
// separate out keyColumns so they are consistently positioned in the
// CTE since they won't necessarily be ordered in the source map
Expand Down
12 changes: 12 additions & 0 deletions test/statement/insert.js
Expand Up @@ -183,5 +183,17 @@ describe('Insert', function () {
assert.isOk(err);
}
});

it('should throw when formatting a deep insert with bad definitions', function () {
const x = new Insert(
source,
{
field1: 'value1',
not_a_junction: 'q'
}
);

assert.throws(x.format);
});
});
});
33 changes: 33 additions & 0 deletions test/table/deep-insert.js
Expand Up @@ -56,4 +56,37 @@ describe('deep insert', function () {
const order2 = orders.find(o => o.user_id === 6);
assert.equal(order2.notes, 'deep insert test 2');
});

it('errors on attempting to deep insert multiple records', function () {
return db.products.insert([{
name: 'something',
orders: [{
product_id: undefined,
user_id: 5,
notes: 'deep insert test 1'
}, {
product_id: undefined,
user_id: 6,
notes: 'deep insert test 2'
}]
}, {
name: 'something else',
orders: [{
product_id: undefined,
user_id: 6,
notes: 'deep insert test 3'
}]
}])
.then(() => { assert.fail(); })
.catch(() => {});
});

it('errors if a junction is erroneously detected', function () {
return db.products.insert([{
name: 'something',
not_a_column: 'junction false positive'
}])
.then(() => { assert.fail(); })
.catch(() => {});
});
});

0 comments on commit 8ff4045

Please sign in to comment.