diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97b6573d77e4..9f8f18d618c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: strategy: fail-fast: false matrix: - ts-version: ["3.9", "4.0", "4.1"] + ts-version: ["3.9", "4.0", "4.1", "4.2", "4.3", "4.4", "4.5"] name: TS Typings (${{ matrix.ts-version }}) runs-on: ubuntu-latest steps: @@ -204,7 +204,7 @@ jobs: test-mysql-mariadb, test-mssql, ] - if: github.event_name == 'push' && github.ref == 'refs/heads/v6' + if: github.event_name == 'push' && (github.ref == 'refs/heads/v6' || github.ref == 'refs/heads/v6-alpha') env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} NPM_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/README.md b/README.md index 3878c9b9e336..f8325b1f34de 100644 --- a/README.md +++ b/README.md @@ -67,3 +67,4 @@ If you have security issues to report, please refer to our [Responsible Disclosu - [English](https://sequelize.org/master) (OFFICIAL) - [中文文档](https://github.com/demopark/sequelize-docs-Zh-CN) (UNOFFICIAL) + diff --git a/lib/dialects/abstract/query-generator.js b/lib/dialects/abstract/query-generator.js index 5a438cd3268b..5563771332ae 100644 --- a/lib/dialects/abstract/query-generator.js +++ b/lib/dialects/abstract/query-generator.js @@ -180,14 +180,33 @@ class QueryGenerator { let onDuplicateKeyUpdate = ''; + // `options.updateOnDuplicate` is the list of field names to update if a duplicate key is hit during the insert. It + // contains just the field names. This option is _usually_ explicitly set by the corresponding query-interface + // upsert function. if (this._dialect.supports.inserts.updateOnDuplicate && options.updateOnDuplicate) { if (this._dialect.supports.inserts.updateOnDuplicate == ' ON CONFLICT DO UPDATE SET') { // postgres / sqlite // If no conflict target columns were specified, use the primary key names from options.upsertKeys const conflictKeys = options.upsertKeys.map(attr => this.quoteIdentifier(attr)); const updateKeys = options.updateOnDuplicate.map(attr => `${this.quoteIdentifier(attr)}=EXCLUDED.${this.quoteIdentifier(attr)}`); - onDuplicateKeyUpdate = ` ON CONFLICT (${conflictKeys.join(',')}) DO UPDATE SET ${updateKeys.join(',')}`; + onDuplicateKeyUpdate = ` ON CONFLICT (${conflictKeys.join(',')})`; + // if update keys are provided, then apply them here. if there are no updateKeys provided, then do not try to + // do an update. Instead, fall back to DO NOTHING. + onDuplicateKeyUpdate += _.isEmpty(updateKeys.length) ? ' DO NOTHING ' : ` DO UPDATE SET ${updateKeys.join(',')}`; } else { const valueKeys = options.updateOnDuplicate.map(attr => `${this.quoteIdentifier(attr)}=VALUES(${this.quoteIdentifier(attr)})`); + // the rough equivalent to ON CONFLICT DO NOTHING in mysql, etc is ON DUPLICATE KEY UPDATE id = id + // So, if no update values were provided, fall back to the identifier columns provided in the upsertKeys array. + // This will be the primary key in most cases, but it could be some other constraint. + if (_.isEmpty(valueKeys) && options.upsertKeys) { + valueKeys.push(...options.upsertKeys.map(attr => `${this.quoteIdentifier(attr)}=${this.quoteIdentifier(attr)}`)); + } + + // edge case... but if for some reason there were no valueKeys, and there were also no upsertKeys... then we + // can no longer build the requested query without a syntax error. Let's throw something more graceful here + // so the devs know what the problem is. + if (_.isEmpty(valueKeys)) { + throw new Error('No update values found for ON DUPLICATE KEY UPDATE clause, and no identifier fields could be found to use instead.'); + } onDuplicateKeyUpdate += `${this._dialect.supports.inserts.updateOnDuplicate} ${valueKeys.join(',')}`; } } @@ -286,6 +305,9 @@ class QueryGenerator { tuples.push(`(${values.join(',')})`); } + // `options.updateOnDuplicate` is the list of field names to update if a duplicate key is hit during the insert. It + // contains just the field names. This option is _usually_ explicitly set by the corresponding query-interface + // upsert function. if (this._dialect.supports.inserts.updateOnDuplicate && options.updateOnDuplicate) { if (this._dialect.supports.inserts.updateOnDuplicate == ' ON CONFLICT DO UPDATE SET') { // postgres / sqlite // If no conflict target columns were specified, use the primary key names from options.upsertKeys diff --git a/lib/dialects/mssql/query-generator.js b/lib/dialects/mssql/query-generator.js index 94e643ee0b23..3e44de52da72 100644 --- a/lib/dialects/mssql/query-generator.js +++ b/lib/dialects/mssql/query-generator.js @@ -449,7 +449,7 @@ class MSSQLQueryGenerator extends AbstractQueryGenerator { //IDENTITY_INSERT Condition identityAttrs.forEach(key => { - if (updateValues[key] && updateValues[key] !== null) { + if (insertValues[key] && insertValues[key] !== null) { needIdentityInsertWrapper = true; /* * IDENTITY_INSERT Column Cannot be updated, only inserted @@ -501,16 +501,18 @@ class MSSQLQueryGenerator extends AbstractQueryGenerator { } // Remove the IDENTITY_INSERT Column from update - const updateSnippet = updateKeys.filter(key => !identityAttrs.includes(key)) + const filteredUpdateClauses = updateKeys.filter(key => !identityAttrs.includes(key)) .map(key => { const value = this.escape(updateValues[key]); key = this.quoteIdentifier(key); return `${targetTableAlias}.${key} = ${value}`; - }).join(', '); + }); + const updateSnippet = filteredUpdateClauses.length > 0 ? `WHEN MATCHED THEN UPDATE SET ${filteredUpdateClauses.join(', ')}` : ''; const insertSnippet = `(${insertKeysQuoted}) VALUES(${insertValuesEscaped})`; + let query = `MERGE INTO ${tableNameQuoted} WITH(HOLDLOCK) AS ${targetTableAlias} USING (${sourceTableQuery}) AS ${sourceTableAlias}(${insertKeysQuoted}) ON ${joinCondition}`; - query += ` WHEN MATCHED THEN UPDATE SET ${updateSnippet} WHEN NOT MATCHED THEN INSERT ${insertSnippet} OUTPUT $action, INSERTED.*;`; + query += ` ${updateSnippet} WHEN NOT MATCHED THEN INSERT ${insertSnippet} OUTPUT $action, INSERTED.*;`; if (needIdentityInsertWrapper) { query = `SET IDENTITY_INSERT ${tableNameQuoted} ON; ${query} SET IDENTITY_INSERT ${tableNameQuoted} OFF;`; } diff --git a/lib/dialects/mssql/query.js b/lib/dialects/mssql/query.js index bbead007705a..f35c43d948e7 100644 --- a/lib/dialects/mssql/query.js +++ b/lib/dialects/mssql/query.js @@ -216,6 +216,11 @@ class Query extends AbstractQuery { return data; } if (this.isUpsertQuery()) { + // if this was an upsert and no data came back, that means the record exists, but the update was a noop. + // return the current instance and mark it as an "not an insert". + if (data && data.length === 0) { + return [this.instance || data, false]; + } this.handleInsertQuery(data); return [this.instance || data, data[0].$action === 'INSERT']; } diff --git a/lib/dialects/mysql/query-interface.js b/lib/dialects/mysql/query-interface.js index bcdec9d2e5a7..4c7a43108e70 100644 --- a/lib/dialects/mysql/query-interface.js +++ b/lib/dialects/mysql/query-interface.js @@ -46,6 +46,7 @@ class MySQLQueryInterface extends QueryInterface { options.type = QueryTypes.UPSERT; options.updateOnDuplicate = Object.keys(updateValues); + options.upsertKeys = Object.values(options.model.primaryKeys).map(item => item.field); const model = options.model; const sql = this.queryGenerator.insertQuery(tableName, insertValues, model.rawAttributes, options); diff --git a/lib/dialects/postgres/query.js b/lib/dialects/postgres/query.js index 3640bda38648..8d63110340b2 100644 --- a/lib/dialects/postgres/query.js +++ b/lib/dialects/postgres/query.js @@ -267,7 +267,7 @@ class Query extends AbstractQuery { if (this.instance && this.instance.dataValues) { // If we are creating an instance, and we get no rows, the create failed but did not throw. // This probably means a conflict happened and was ignored, to avoid breaking a transaction. - if (this.isInsertQuery() && rowCount === 0) { + if (this.isInsertQuery() && !this.isUpsertQuery() && rowCount === 0) { throw new sequelizeErrors.EmptyResultError(); } diff --git a/lib/model.js b/lib/model.js index 9960422d2383..16bfd90d697e 100644 --- a/lib/model.js +++ b/lib/model.js @@ -1672,7 +1672,7 @@ class Model { * @param {object} [options.having] Having options * @param {string} [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only) * @param {boolean|Error} [options.rejectOnEmpty=false] Throws an error when no records found - * @param {boolean} [options.dotNotation] Allows including tables having the same attribute/column names - which have a dot in them. + * @param {boolean} [options.dotNotation] Allows including tables having the same attribute/column names - which have a dot in them. * * @see * {@link Sequelize#query} @@ -2438,7 +2438,7 @@ class Model { * @param {object} values hash of values to upsert * @param {object} [options] upsert options * @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 {Array} [options.fields=Object.keys(this.attributes)] The fields to update if the record already exists. Defaults to all changed fields. If none of the specified fields are present on the provided `values` object, an insert will still be attempted, but duplicate key conflicts will be ignored. * @param {boolean} [options.hooks=true] Run before / after upsert hooks? * @param {boolean} [options.returning=true] If true, fetches back auto generated values * @param {Transaction} [options.transaction] Transaction to run query under @@ -3315,6 +3315,15 @@ class Model { } return f; }); + } else if (fields && typeof fields === 'object') { + fields = Object.keys(fields).reduce((rawFields, f) => { + if (this.rawAttributes[f] && this.rawAttributes[f].field && this.rawAttributes[f].field !== f) { + rawFields[this.rawAttributes[f].field] = fields[f]; + } else { + rawFields[f] = fields[f]; + } + return rawFields; + }, {}); } this._injectScope(options); diff --git a/package.json b/package.json index 383f7d52cd75..568a36837a37 100644 --- a/package.json +++ b/package.json @@ -185,7 +185,11 @@ "@semantic-release/github" ], "branches": [ - "v6" + "v6", + { + "name": "v6-alpha", + "prerelease": "alpha" + } ] }, "publishConfig": { diff --git a/test/integration/instance/increment.test.js b/test/integration/instance/increment.test.js index ab1b15224823..d8345c25af9a 100644 --- a/test/integration/instance/increment.test.js +++ b/test/integration/instance/increment.test.js @@ -28,6 +28,7 @@ describe(Support.getTestDialectTeaser('Instance'), () => { touchedAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, aNumber: { type: DataTypes.INTEGER }, bNumber: { type: DataTypes.INTEGER }, + cNumber: { type: DataTypes.INTEGER, field: 'CNumberColumn' }, aDate: { type: DataTypes.DATE }, validateTest: { @@ -57,7 +58,7 @@ describe(Support.getTestDialectTeaser('Instance'), () => { describe('increment', () => { beforeEach(async function() { - await this.User.create({ id: 1, aNumber: 0, bNumber: 0 }); + await this.User.create({ id: 1, aNumber: 0, bNumber: 0, cNumber: 0 }); }); if (current.dialect.supports.transactions) { @@ -150,6 +151,27 @@ describe(Support.getTestDialectTeaser('Instance'), () => { expect(user3.bNumber).to.be.equal(2); }); + it('single value should work when field name is different from database column name', async function() { + const user = await this.User.findByPk(1); + await user.increment('cNumber'); + const user2 = await this.User.findByPk(1); + expect(user2.cNumber).to.be.equal(1); + }); + + it('array should work when field name is different from database column name', async function() { + const user = await this.User.findByPk(1); + await user.increment(['cNumber']); + const user2 = await this.User.findByPk(1); + expect(user2.cNumber).to.be.equal(1); + }); + + it('key value should work when field name is different from database column name', async function() { + const user = await this.User.findByPk(1); + await user.increment({ cNumber: 1 }); + const user2 = await this.User.findByPk(1); + expect(user2.cNumber).to.be.equal(1); + }); + it('with timestamps set to true', async function() { const User = this.sequelize.define('IncrementUser', { aNumber: DataTypes.INTEGER diff --git a/test/integration/model/upsert.test.js b/test/integration/model/upsert.test.js index bfbab5e27e13..10001ce568ca 100644 --- a/test/integration/model/upsert.test.js +++ b/test/integration/model/upsert.test.js @@ -311,6 +311,30 @@ describe(Support.getTestDialectTeaser('Model'), () => { clock.restore(); }); + it('falls back to a noop if no update values are found in the upsert data', async function() { + const User = this.sequelize.define('user', { + username: DataTypes.STRING, + email: { + type: DataTypes.STRING, + field: 'email_address', + defaultValue: 'xxx@yyy.zzz' + } + }, { + // note, timestamps: false is important here because this test is attempting to see what happens + // if there are NO updatable fields (including timestamp values). + timestamps: false + }); + + await User.sync({ force: true }); + // notice how the data does not actually have the update fields. + await User.upsert({ id: 42, username: 'jack' }, { fields: ['email'] }); + await User.upsert({ id: 42, username: 'jill' }, { fields: ['email'] }); + const user = await User.findByPk(42); + // just making sure the user exists, i.e. the insert happened. + expect(user).to.be.ok; + expect(user.username).to.equal('jack'); // second upsert should not have updated username. + }); + it('does not update using default values', async function() { await this.User.create({ id: 42, username: 'john', baz: 'new baz value' }); const user0 = await this.User.findByPk(42); diff --git a/types/lib/associations/belongs-to-many.d.ts b/types/lib/associations/belongs-to-many.d.ts index c82e9294d971..b7106ffb197b 100644 --- a/types/lib/associations/belongs-to-many.d.ts +++ b/types/lib/associations/belongs-to-many.d.ts @@ -9,8 +9,7 @@ import { Model, ModelCtor, ModelType, - Transactionable, - WhereOptions, + Transactionable } from '../model'; import { Association, AssociationScope, ForeignKeyOptions, ManyToManyOptions, MultiAssociationAccessors } from './base'; @@ -304,7 +303,7 @@ export interface BelongsToManyCreateAssociationMixinOptions extends CreateOption * @see Instance */ export type BelongsToManyCreateAssociationMixin = ( - values?: Model['_creationAttributes'], + values?: TModel['_creationAttributes'], options?: BelongsToManyCreateAssociationMixinOptions ) => Promise; diff --git a/types/lib/associations/has-many.d.ts b/types/lib/associations/has-many.d.ts index d98a96485af3..26b62444b13f 100644 --- a/types/lib/associations/has-many.d.ts +++ b/types/lib/associations/has-many.d.ts @@ -6,7 +6,7 @@ import { InstanceUpdateOptions, Model, ModelCtor, - Transactionable, + Transactionable } from '../model'; import { Association, ManyToManyOptions, MultiAssociationAccessors } from './base'; @@ -210,7 +210,7 @@ export interface HasManyCreateAssociationMixinOptions extends CreateOptions * @see Instance */ export type HasManyCreateAssociationMixin = ( - values?: Model['_creationAttributes'], + values?: TModel['_creationAttributes'], options?: HasManyCreateAssociationMixinOptions ) => Promise; diff --git a/types/test/typescriptDocs/ModelInit.ts b/types/test/typescriptDocs/ModelInit.ts index 163cb8ec88ee..a8848a0bc1cd 100644 --- a/types/test/typescriptDocs/ModelInit.ts +++ b/types/test/typescriptDocs/ModelInit.ts @@ -2,17 +2,9 @@ * Keep this file in sync with the code in the "Usage" section in typescript.md */ import { - Sequelize, - Model, - ModelDefined, - DataTypes, - HasManyGetAssociationsMixin, - HasManyAddAssociationMixin, - HasManyHasAssociationMixin, - Association, - HasManyCountAssociationsMixin, - HasManyCreateAssociationMixin, - Optional, + Association, DataTypes, HasManyAddAssociationMixin, HasManyCountAssociationsMixin, + HasManyCreateAssociationMixin, HasManyGetAssociationsMixin, HasManyHasAssociationMixin, Model, + ModelDefined, Optional, Sequelize } from "sequelize"; const sequelize = new Sequelize("mysql://root:asd123@localhost:3306/mydb"); @@ -59,6 +51,7 @@ interface ProjectAttributes { id: number; ownerId: number; name: string; + description?: string; } interface ProjectCreationAttributes extends Optional {} @@ -114,6 +107,10 @@ Project.init( type: new DataTypes.STRING(128), allowNull: false, }, + description: { + type: new DataTypes.STRING(128), + allowNull: true, + }, }, { sequelize, @@ -204,6 +201,7 @@ async function doStuffWithUser() { const project = await newUser.createProject({ name: "first!", + ownerId: 123, }); const ourUser = await User.findByPk(1, {