diff --git a/lib/dialects/mssql/query-generator.js b/lib/dialects/mssql/query-generator.js index 7f52f89aa5ca..9a87164dfd79 100644 --- a/lib/dialects/mssql/query-generator.js +++ b/lib/dialects/mssql/query-generator.js @@ -988,6 +988,8 @@ class MSSQLQueryGenerator extends AbstractQueryGenerator { } if (options.limit || options.offset) { + // TODO: document why this is adding the primary key of the model in ORDER BY + // if options.include is set if (!options.order || options.order.length === 0 || options.include && orders.subQueryOrder.length === 0) { const tablePkFragment = `${this.quoteTable(options.tableAs || model.name)}.${this.quoteIdentifier(model.primaryKeyField)}`; if (!options.order || options.order.length === 0) { diff --git a/lib/operators.ts b/lib/operators.ts index 2329e0ac747e..29a161468997 100644 --- a/lib/operators.ts +++ b/lib/operators.ts @@ -478,6 +478,11 @@ interface OpTypes { readonly values: unique symbol; } +// Note: These symbols are registered in the Global Symbol Registry +// to counter bugs when two different versions of this library are loaded +// Source issue: https://github.com/sequelize/sequelize/issues/8663 +// This is not an endorsement of having two different versions of the library loaded at the same time, +// a lot more is going to silently break if you do this. export const Op: OpTypes = { eq: Symbol.for('eq'), ne: Symbol.for('ne'), diff --git a/test/support.js b/test/support.js index 7a3238a82a68..5f8337e64d6a 100644 --- a/test/support.js +++ b/test/support.js @@ -268,6 +268,21 @@ const Support = { isDeepEqualToOneOf(actual, expectedOptions) { return expectedOptions.some(expected => isDeepStrictEqual(actual, expected)); }, + + /** + * Reduces insignificant whitespace from SQL string. + * + * @param {string} sql the SQL string + * @returns {string} the SQL string with insignificant whitespace removed. + */ + minifySql(sql) { + // replace all consecutive whitespaces with a single plain space character + return sql.replace(/\s+/g, ' ') + // remove space before coma + .replace(/ ,/g, ',') + // remove whitespace at start & end + .trim(); + }, }; if (global.beforeEach) { diff --git a/test/unit/sql/select.test.js b/test/unit/sql/select.test.js index d3346f953cc2..3a30bf91be9a 100644 --- a/test/unit/sql/select.test.js +++ b/test/unit/sql/select.test.js @@ -16,10 +16,10 @@ const Op = Support.Sequelize.Op; describe(Support.getTestDialectTeaser('SQL'), () => { describe('select', () => { - const testsql = function (options, expectation) { + const testsql = function (options, expectation, testFunction = it) { const model = options.model; - it(util.inspect(options, { depth: 2 }), () => { + testFunction(util.inspect(options, { depth: 2 }), () => { return expectsql( sql.selectQuery( options.table || model && model.getTableName(), @@ -31,6 +31,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => { }); }; + testsql.only = (options, expectation) => testsql(options, expectation, it.only); + testsql({ table: 'User', attributes: [ @@ -300,6 +302,32 @@ describe(Support.getTestDialectTeaser('SQL'), () => { }) AS [user] LEFT OUTER JOIN [post] AS [POSTS] ON [user].[id_user] = [POSTS].[user_id] ORDER BY [user].[last_name] ASC;`, }); + // By default, SELECT with include of a multi association & limit will be ran as a subQuery + // This checks the result when the query is forced to be ran without a subquery + testsql({ + table: User.getTableName(), + model: User, + include, + attributes: [ + ['id_user', 'id'], + 'email', + ['first_name', 'firstName'], + ['last_name', 'lastName'], + ], + order: [['[last_name]'.replace(/\[/g, Support.sequelize.dialect.TICK_CHAR_LEFT).replace(/\]/g, Support.sequelize.dialect.TICK_CHAR_RIGHT), 'ASC']], + limit: 30, + offset: 10, + hasMultiAssociation: true, // must be set only for mssql dialect here + subQuery: false, + }, { + default: Support.minifySql(`SELECT [user].[id_user] AS [id], [user].[email], [user].[first_name] AS [firstName], [user].[last_name] AS [lastName], [POSTS].[id] AS [POSTS.id], [POSTS].[title] AS [POSTS.title] + FROM [users] AS [user] LEFT OUTER JOIN [post] AS [POSTS] + ON [user].[id_user] = [POSTS].[user_id] + ORDER BY [user].[last_name] ASC + ${sql.addLimitAndOffset({ limit: 30, offset: 10, order: [['last_name', 'ASC']], include }, User)}; + `), + }); + const nestedInclude = Model._validateIncludedElements({ include: [{ attributes: ['title'], diff --git a/types/lib/model.d.ts b/types/lib/model.d.ts index b5f5982dbf92..d74661797bef 100644 --- a/types/lib/model.d.ts +++ b/types/lib/model.d.ts @@ -555,10 +555,37 @@ export interface FindOptions group?: GroupOption; /** - * Limit the results + * Limits how many items will be retrieved by the operation. + * + * If `limit` and `include` are used together, Sequelize will turn the `subQuery` option on by default. + * This is done to ensure that `limit` only impacts the Model on the same level as the `limit` option. + * + * You can disable this behavior by explicitly setting `subQuery: false`, however `limit` will then + * affect the total count of returned values, including eager-loaded associations, instead of just one table. + * + * @example + * // in the following query, `limit` only affects the "User" model. + * // This will return 2 users, each including all of their projects. + * User.findAll({ + * limit: 2, + * include: [User.associations.projects], + * }); + * + * @example + * // in the following query, `limit` affects the total number of returned values, eager-loaded associations included. + * // This may return 2 users, each with one project, + * // or 1 user with 2 projects. + * User.findAll({ + * limit: 2, + * include: [User.associations.projects], + * subQuery: false, + * }); */ limit?: number; + // TODO: document this - this is an undocumented property but it exists and there are tests for it. + groupedLimit?: unknown; + /** * Skip the results; */ @@ -589,7 +616,10 @@ export interface FindOptions having?: WhereOptions; /** - * Use sub queries (internal) + * Use sub queries (internal). + * + * If unspecified, this will `true` by default if `limit` is specified, and `false` otherwise. + * See {@link FindOptions#limit} for more information. */ subQuery?: boolean; }