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

feat(upsert): return upserted record with options.returning=true #8924

Merged
merged 1 commit into from
Jan 22, 2018
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
1 change: 0 additions & 1 deletion lib/dialects/mssql/query-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,6 @@ const QueryGenerator = {
const tableNameQuoted = this.quoteTable(tableName);
let needIdentityInsertWrapper = false;


//Obtain primaryKeys, uniquekeys and identity attrs from rawAttributes as model is not passed
for (const key in model.rawAttributes) {
if (model.rawAttributes[key].primaryKey) {
Expand Down
7 changes: 1 addition & 6 deletions lib/dialects/mssql/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,7 @@ class Query extends AbstractQuery {
} else if (this.isSelectQuery()) {
result = this.handleSelectQuery(data);
} else if (this.isUpsertQuery()) {
//Use the same return value as that of MySQL & Postgres
if (data[0].$action === 'INSERT') {
result = 1;
} else {
result = 2;
}
result = data[0];
} else if (this.isCallQuery()) {
result = data[0];
} else if (this.isBulkUpdateQuery()) {
Expand Down
28 changes: 16 additions & 12 deletions lib/dialects/postgres/query-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,34 +345,38 @@ const QueryGenerator = {
return `ALTER TABLE ${this.quoteTable(tableName)} RENAME COLUMN ${attrString.join(', ')};`;
},

fn(fnName, tableName, body, returns, language) {
fn(fnName, tableName, parameters, body, returns, language) {
fnName = fnName || 'testfunc';
language = language || 'plpgsql';
returns = returns || 'SETOF ' + this.quoteTable(tableName);
returns = returns ? `RETURNS ${returns}` : '';
parameters = parameters || '';

return `CREATE OR REPLACE FUNCTION pg_temp.${fnName}() RETURNS ${returns} AS $func$ BEGIN ${body} END; $func$ LANGUAGE ${language}; SELECT * FROM pg_temp.${fnName}();`;
return `CREATE OR REPLACE FUNCTION pg_temp.${fnName}(${parameters}) ${returns} AS $func$ BEGIN ${body} END; $func$ LANGUAGE ${language}; SELECT * FROM pg_temp.${fnName}();`;
},

exceptionFn(fnName, tableName, main, then, when, returns, language) {
exceptionFn(fnName, tableName, parameters, main, then, when, returns, language) {
when = when || 'unique_violation';

const body = `${main} EXCEPTION WHEN ${when} THEN ${then};`;

return this.fn(fnName, tableName, body, returns, language);
return this.fn(fnName, tableName, parameters, body, returns, language);
},

upsertQuery(tableName, insertValues, updateValues, where, model, options) {
const insert = this.insertQuery(tableName, insertValues, model.rawAttributes, options);
const update = this.updateQuery(tableName, updateValues, where, options, model.rawAttributes);
const primaryField = this.quoteIdentifier(model.primaryKeyField);

let insert = this.insertQuery(tableName, insertValues, model.rawAttributes, options);
let update = this.updateQuery(tableName, updateValues, where, options, model.rawAttributes);

insert = insert.replace('RETURNING *', `RETURNING ${primaryField} INTO primary_key`);
update = update.replace('RETURNING *', `RETURNING ${primaryField} INTO primary_key`);

// The numbers here are selected to match the number of affected rows returned by MySQL
return this.exceptionFn(
'sequelize_upsert',
tableName,
insert + ' RETURN 1;',
update + '; RETURN 2',
'unique_violation',
'integer'
'OUT created boolean, OUT primary_key text',
`${insert} created := true;`,
`${update}; created := false`
);
},

Expand Down
3 changes: 1 addition & 2 deletions lib/dialects/postgres/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,11 @@ class Query extends AbstractQuery {
if (!this.options.returning) {
return parseInt(rowCount, 10);
}

return this.handleSelectQuery(rows);
} else if (QueryTypes.BULKDELETE === this.options.type) {
return parseInt(rowCount, 10);
} else if (this.isUpsertQuery()) {
return rows[0].sequelize_upsert;
return rows[0];
} else if (this.isInsertQuery() || this.isUpdateQuery()) {
if (this.instance && this.instance.dataValues) {
for (const key in rows[0]) {
Expand Down
2 changes: 2 additions & 0 deletions lib/dialects/sqlite/query-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ exports.addConstraint = addConstraint;
*
* @param {String} tableName
* @param {Object} options Query Options
*
* @private
* @returns {Promise}
*/
function getForeignKeyReferencesForTable(tableName, options) {
Expand Down
19 changes: 14 additions & 5 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -2215,22 +2215,25 @@ class Model {
* @param {Boolean} [options.validate=true] Run validations before the row is inserted
* @param {Array} [options.fields=Object.keys(this.attributes)] The fields to insert / update. Defaults to all changed fields
* @param {Boolean} [options.hooks=true] Run before / after upsert hooks?
* @param {Boolean} [options.returning=false] Append RETURNING * to get back auto generated values (Postgres only)
* @param {Transaction} [options.transaction] Transaction to run query under
* @param {Function} [options.logging=false] A function that gets executed while running the query to log the sql.
* @param {Boolean} [options.benchmark=false] Pass query execution time in milliseconds as second argument to logging function (options.logging).
* @param {String} [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only)
*
* @return {Promise<created>} Returns a boolean indicating whether the row was created or updated.
* @return {Promise<created>} Returns a boolean indicating whether the row was created or updated. For Postgres/MSSQL with (options.returning=true), it returns record and created boolean with signature `<Model, created>`.
*/
static upsert(values, options) {
options = _.extend({
hooks: true
options = Object.assign({
hooks: true,
returning: false
}, Utils.cloneDeep(options || {}));

options.model = this;

const createdAtAttr = this._timestampAttributes.createdAt;
const updatedAtAttr = this._timestampAttributes.updatedAt;
const hadPrimary = this.primaryKeyField in values || this.primaryKeyAttribute in values;
const hasPrimary = this.primaryKeyField in values || this.primaryKeyAttribute in values;
const instance = this.build(values);

if (!options.fields) {
Expand All @@ -2256,7 +2259,7 @@ class Model {

// Build adds a null value for the primary key, if none was given by the user.
// We need to remove that because of some Postgres technicalities.
if (!hadPrimary && this.primaryKeyAttribute && !this.rawAttributes[this.primaryKeyAttribute].defaultValue) {
if (!hasPrimary && this.primaryKeyAttribute && !this.rawAttributes[this.primaryKeyAttribute].defaultValue) {
delete insertValues[this.primaryKeyField];
delete updateValues[this.primaryKeyField];
}
Expand All @@ -2267,6 +2270,12 @@ class Model {
}
}).then(() => {
return this.QueryInterface.upsert(this.getTableName(options), insertValues, updateValues, instance.where(), this, options);
}).spread((created, primaryKey) => {
if (options.returning === true && primaryKey) {
return this.findById(primaryKey, options).then(record => [record, created]);
}

return created;
}).tap(result => {
if (options.hooks) {
return this.runHooks('afterUpsert', result, options);
Expand Down
45 changes: 34 additions & 11 deletions lib/query-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,21 @@ class QueryInterface {
});
}

upsert(tableName, valuesByField, updateValues, where, model, options) {
/**
* Upsert
*
* @param {String} tableName
* @param {Object} insertValues values to be inserted, mapped to field name
* @param {Object} updateValues values to be updated, mapped to field name
* @param {Object} where various conditions
* @param {Model} model
* @param {Object} options
*
* @returns {Promise<created, primaryKey>}
*/
upsert(tableName, insertValues, updateValues, where, model, options) {
const wheres = [];
const attributes = Object.keys(valuesByField);
const attributes = Object.keys(insertValues);
let indexes = [];
let indexFields;

Expand Down Expand Up @@ -938,7 +950,7 @@ class QueryInterface {
if (_.intersection(attributes, index).length === index.length) {
where = {};
for (const field of index) {
where[field] = valuesByField[field];
where[field] = insertValues[field];
}
wheres.push(where);
}
Expand All @@ -949,15 +961,26 @@ class QueryInterface {
options.type = QueryTypes.UPSERT;
options.raw = true;

const sql = this.QueryGenerator.upsertQuery(tableName, valuesByField, updateValues, where, model, options);
return this.sequelize.query(sql, options).then(rowCount => {
if (rowCount === undefined) {
return rowCount;
const sql = this.QueryGenerator.upsertQuery(tableName, insertValues, updateValues, where, model, options);
return this.sequelize.query(sql, options).then(result => {
switch (this.sequelize.options.dialect) {
case 'postgres':
return [result.created, result.primary_key];

case 'mssql':
return [
result.$action === 'INSERT',
result[model.primaryKeyField]
];

// MySQL returns 1 for inserted, 2 for updated
// http://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html.
case 'mysql':
return [result === 1, undefined];

default:
return [result, undefined];
}

// MySQL returns 1 for inserted, 2 for updated http://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html. Postgres has been modded to do the same

return rowCount === 1;
});
}

Expand Down
89 changes: 81 additions & 8 deletions test/integration/model/upsert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ describe(Support.getTestDialectTeaser('Model'), () => {
this.clock = sinon.useFakeTimers();
});

after(function() {
this.clock.restore();
});

beforeEach(function() {
this.clock.reset();
});

beforeEach(function() {
this.User = this.sequelize.define('user', {
username: DataTypes.STRING,
Expand Down Expand Up @@ -50,10 +58,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
return this.sequelize.sync({ force: true });
});

after(function() {
this.clock.restore();
});

if (current.dialect.supports.upserts) {
describe('upsert', () => {
it('works with upsert on id', function() {
Expand Down Expand Up @@ -154,7 +158,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(created2).to.be.ok;
}


this.clock.tick(1000);
// Update the first one
return User.upsert({ a: 'a', b: 'b', username: 'doe' });
Expand Down Expand Up @@ -201,7 +204,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(created).to.be.ok;
}


this.clock.tick(1000);
return this.User.upsert({ id: 42, username: 'doe', blob: new Buffer('andrea') });
}).then(function(created) {
Expand Down Expand Up @@ -250,7 +252,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(created).to.be.ok;
}


this.clock.tick(1000);
return this.ModelWithFieldPK.upsert({ userId: 42, foo: 'second' });
}).then(function(created) {
Expand All @@ -274,7 +275,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(created).to.be.ok;
}


this.clock.tick(1000);
return this.User.upsert({ id: 42, username: 'doe', foo: this.sequelize.fn('upper', 'mixedCase2') });
}).then(function(created) {
Expand Down Expand Up @@ -525,6 +525,79 @@ describe(Support.getTestDialectTeaser('Model'), () => {
});
});
}

if (current.dialect.supports.returnValues) {
describe('with returning option', () => {
it('works with upsert on id', function() {
return this.User.upsert({ id: 42, username: 'john' }, { returning: true }).spread((user, created) => {
expect(user.get('id')).to.equal(42);
expect(user.get('username')).to.equal('john');
expect(created).to.be.true;

return this.User.upsert({ id: 42, username: 'doe' }, { returning: true });
}).spread((user, created) => {
expect(user.get('id')).to.equal(42);
expect(user.get('username')).to.equal('doe');
expect(created).to.be.false;
});
});

it('works for table with custom primary key field', function() {
const User = this.sequelize.define('User', {
id: {
type: DataTypes.INTEGER,
autoIncrement: true,
primaryKey: true,
field: 'id_the_primary'
},
username: {
type: DataTypes.STRING
}
});

return User.sync({ force: true }).then(() => {
return User.upsert({ id: 42, username: 'john' }, { returning: true });
}).spread((user, created) => {
expect(user.get('id')).to.equal(42);
expect(user.get('username')).to.equal('john');
expect(created).to.be.true;

return User.upsert({ id: 42, username: 'doe' }, { returning: true });
}).spread((user, created) => {
expect(user.get('id')).to.equal(42);
expect(user.get('username')).to.equal('doe');
expect(created).to.be.false;
});
});

it('works for non incrementing primaryKey', function() {
const User = this.sequelize.define('User', {
id: {
type: DataTypes.STRING,
primaryKey: true,
field: 'id_the_primary'
},
username: {
type: DataTypes.STRING
}
});

return User.sync({ force: true }).then(() => {
return User.upsert({ id: 'surya', username: 'john' }, { returning: true });
}).spread((user, created) => {
expect(user.get('id')).to.equal('surya');
expect(user.get('username')).to.equal('john');
expect(created).to.be.true;

return User.upsert({ id: 'surya', username: 'doe' }, { returning: true });
}).spread((user, created) => {
expect(user.get('id')).to.equal('surya');
expect(user.get('username')).to.equal('doe');
expect(created).to.be.false;
});
});
});
}
});
}
});