docs: clarify how the limit option works (#13985)

lib/dialects/mssql/query-generator.js

@@ -966,7 +966,9 @@ class MSSQLQueryGenerator extends AbstractQueryGenerator {
     }
 
     if (options.limit || options.offset) {
-      if (!options.order || !options.order.length || options.include && !orders.subQueryOrder.length) {
+      // 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) {
           fragment += ` ORDER BY ${tablePkFragment}`;

lib/operators.ts

@@ -478,7 +478,12 @@ interface OpTypes {
   readonly values: unique symbol;
 }
 
-const Op: OpTypes = {
+// 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'),
   gte: Symbol.for('gte'),

test/support.js

@@ -248,6 +248,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();
   }
 };
 

test/unit/sql/select.test.js

@@ -15,10 +15,10 @@ const Support = require('../support'),
 
 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(),
@@ -30,6 +30,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
       });
     };
 
+    testsql.only = (options, expectation) => testsql(options, expectation, it.only);
+
     testsql({
       table: 'User',
       attributes: [
@@ -299,6 +301,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'],

types/lib/model.d.ts

@@ -555,10 +555,37 @@ export interface FindOptions<TAttributes = any>
   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<TAttributes = any>
   having?: WhereOptions<any>;
 
   /**
-   * 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;
 }