Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/joe223/sequelize
Browse files Browse the repository at this point in the history
  • Loading branch information
joe223 committed Nov 22, 2021
2 parents cd9e384 + 308625d commit f393853
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 28 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Expand Up @@ -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:
Expand Down Expand Up @@ -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 }}
Expand Down
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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)

24 changes: 23 additions & 1 deletion lib/dialects/abstract/query-generator.js
Expand Up @@ -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(',')}`;
}
}
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions lib/dialects/mssql/query-generator.js
Expand Up @@ -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
Expand Down Expand Up @@ -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;`;
}
Expand Down
5 changes: 5 additions & 0 deletions lib/dialects/mssql/query.js
Expand Up @@ -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'];
}
Expand Down
1 change: 1 addition & 0 deletions lib/dialects/mysql/query-interface.js
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/postgres/query.js
Expand Up @@ -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();
}

Expand Down
13 changes: 11 additions & 2 deletions lib/model.js
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion package.json
Expand Up @@ -185,7 +185,11 @@
"@semantic-release/github"
],
"branches": [
"v6"
"v6",
{
"name": "v6-alpha",
"prerelease": "alpha"
}
]
},
"publishConfig": {
Expand Down
24 changes: 23 additions & 1 deletion test/integration/instance/increment.test.js
Expand Up @@ -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: {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions test/integration/model/upsert.test.js
Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions types/lib/associations/belongs-to-many.d.ts
Expand Up @@ -9,8 +9,7 @@ import {
Model,
ModelCtor,
ModelType,
Transactionable,
WhereOptions,
Transactionable
} from '../model';
import { Association, AssociationScope, ForeignKeyOptions, ManyToManyOptions, MultiAssociationAccessors } from './base';

Expand Down Expand Up @@ -304,7 +303,7 @@ export interface BelongsToManyCreateAssociationMixinOptions extends CreateOption
* @see Instance
*/
export type BelongsToManyCreateAssociationMixin<TModel extends Model> = (
values?: Model['_creationAttributes'],
values?: TModel['_creationAttributes'],
options?: BelongsToManyCreateAssociationMixinOptions
) => Promise<TModel>;

Expand Down
4 changes: 2 additions & 2 deletions types/lib/associations/has-many.d.ts
Expand Up @@ -6,7 +6,7 @@ import {
InstanceUpdateOptions,
Model,
ModelCtor,
Transactionable,
Transactionable
} from '../model';
import { Association, ManyToManyOptions, MultiAssociationAccessors } from './base';

Expand Down Expand Up @@ -210,7 +210,7 @@ export interface HasManyCreateAssociationMixinOptions extends CreateOptions<any>
* @see Instance
*/
export type HasManyCreateAssociationMixin<TModel extends Model> = (
values?: Model['_creationAttributes'],
values?: TModel['_creationAttributes'],
options?: HasManyCreateAssociationMixinOptions
) => Promise<TModel>;

Expand Down
20 changes: 9 additions & 11 deletions types/test/typescriptDocs/ModelInit.ts
Expand Up @@ -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");
Expand Down Expand Up @@ -59,6 +51,7 @@ interface ProjectAttributes {
id: number;
ownerId: number;
name: string;
description?: string;
}

interface ProjectCreationAttributes extends Optional<ProjectAttributes, "id"> {}
Expand Down Expand Up @@ -114,6 +107,10 @@ Project.init(
type: new DataTypes.STRING(128),
allowNull: false,
},
description: {
type: new DataTypes.STRING(128),
allowNull: true,
},
},
{
sequelize,
Expand Down Expand Up @@ -204,6 +201,7 @@ async function doStuffWithUser() {

const project = await newUser.createProject({
name: "first!",
ownerId: 123,
});

const ourUser = await User.findByPk(1, {
Expand Down

0 comments on commit f393853

Please sign in to comment.