fix: support array of tableHints and allow joins to use tableHints (#…

…16242)

packages/core/src/dialects/abstract/index.ts

@@ -210,6 +210,7 @@ export type DialectSupports = {
   },
   tmpTableTrigger: boolean,
   indexHints: boolean,
+  tableHints: boolean,
   searchPath: boolean,
   /**
    * This dialect supports E-prefixed strings, e.g. "E'foo'", which
@@ -352,6 +353,7 @@ export abstract class AbstractDialect {
     IREGEXP: false,
     tmpTableTrigger: false,
     indexHints: false,
+    tableHints: false,
     searchPath: false,
     escapeStringConstants: false,
     globalTimeZoneConfig: false,

packages/core/src/dialects/abstract/query-generator-typescript.ts

@@ -15,10 +15,12 @@ import { List } from '../../expression-builders/list.js';
 import { Literal } from '../../expression-builders/literal.js';
 import { Value } from '../../expression-builders/value.js';
 import { Where } from '../../expression-builders/where.js';
+import { IndexHints } from '../../index-hints.js';
 import type { Attributes, Model, ModelStatic } from '../../model.js';
 import { Op } from '../../operators.js';
 import type { BindOrReplacements, Expression, Sequelize } from '../../sequelize.js';
 import { bestGuessDataTypeOfVal } from '../../sql-string.js';
+import { TableHints } from '../../table-hints.js';
 import { isDictionary, isNullish, isPlainObject, isString, rejectInvalidOptions } from '../../utils/check.js';
 import { noOpCol } from '../../utils/deprecations.js';
 import { quoteIdentifier } from '../../utils/dialect.js';
@@ -33,6 +35,7 @@ import type { AbstractQueryGenerator } from './query-generator.js';
 import type {
   AddConstraintQueryOptions,
   GetConstraintSnippetQueryOptions,
+  QuoteTableOptions,
   RemoveConstraintQueryOptions,
   ShowConstraintsQueryOptions,
 } from './query-generator.types.js';
@@ -50,6 +53,7 @@ export interface RemoveIndexQueryOptions {
   cascade?: boolean;
 }
 
+export const QUOTE_TABLE_SUPPORTABLE_OPTIONS = new Set<keyof QuoteTableOptions>(['indexHints', 'tableHints']);
 export const REMOVE_CONSTRAINT_QUERY_SUPPORTABLE_OPTIONS = new Set<keyof RemoveConstraintQueryOptions>(['ifExists', 'cascade']);
 export const REMOVE_INDEX_QUERY_SUPPORTABLE_OPTIONS = new Set<keyof RemoveIndexQueryOptions>(['concurrently', 'ifExists', 'cascade']);
 
@@ -422,9 +426,20 @@ export class AbstractQueryGeneratorTypeScript {
    * Quote table name with optional alias and schema attribution
    *
    * @param param table string or object
-   * @param alias alias name
+   * @param options options
    */
-  quoteTable(param: TableNameOrModel, alias: boolean | string = false): string {
+  quoteTable(param: TableNameOrModel, options?: QuoteTableOptions): string {
+    const QUOTE_TABLE_SUPPORTED_OPTIONS = new Set<keyof QuoteTableOptions>();
+    if (this.dialect.supports.indexHints) {
+      QUOTE_TABLE_SUPPORTED_OPTIONS.add('indexHints');
+    }
+
+    if (this.dialect.supports.tableHints) {
+      QUOTE_TABLE_SUPPORTED_OPTIONS.add('tableHints');
+    }
+
+    rejectInvalidOptions('quoteTable', this.dialect.name, QUOTE_TABLE_SUPPORTABLE_OPTIONS, QUOTE_TABLE_SUPPORTED_OPTIONS, { ...options });
+
     if (isModelStatic(param)) {
       param = param.getTableName();
     }
@@ -435,10 +450,6 @@ export class AbstractQueryGeneratorTypeScript {
       throw new Error('parameters "as" and "name" are not allowed in the first parameter of quoteTable, pass them as the second parameter.');
     }
 
-    if (alias === true) {
-      alias = tableName.tableName;
-    }
-
     let sql = '';
 
     if (this.dialect.supports.schemas) {
@@ -459,8 +470,33 @@ export class AbstractQueryGeneratorTypeScript {
       sql += this.quoteIdentifier(fakeSchemaPrefix + tableName.tableName);
     }
 
-    if (alias) {
-      sql += ` AS ${this.quoteIdentifier(alias)}`;
+    if (options?.alias) {
+      sql += ` AS ${this.quoteIdentifier(options.alias === true ? tableName.tableName : options.alias)}`;
+    }
+
+    if (options?.indexHints) {
+      for (const hint of options.indexHints) {
+        if (IndexHints[hint.type]) {
+          sql += ` ${IndexHints[hint.type]} INDEX (${hint.values.map(indexName => this.quoteIdentifier(indexName)).join(',')})`;
+        } else {
+          throw new Error(`The index hint type "${hint.type}" is invalid or not supported by dialect "${this.dialect.name}".`);
+        }
+      }
+    }
+
+    if (options?.tableHints) {
+      const hints: TableHints[] = [];
+      for (const hint of options.tableHints) {
+        if (TableHints[hint]) {
+          hints.push(TableHints[hint]);
+        } else {
+          throw new Error(`The table hint "${hint}" is invalid or not supported by dialect "${this.dialect.name}".`);
+        }
+      }
+
+      if (hints.length) {
+        sql += ` WITH (${hints.join(', ')})`;
+      }
     }
 
     return sql;

packages/core/src/dialects/abstract/query-generator.js

@@ -1041,8 +1041,8 @@ export class AbstractQueryGenerator extends AbstractQueryGeneratorTypeScript {
 
     mainTable.quotedAs = mainTable.as && this.quoteIdentifier(mainTable.as);
 
-    mainTable.quotedName = !Array.isArray(mainTable.name) ? this.quoteTable(mainTable.name) : tableName.map(t => {
-      return Array.isArray(t) ? this.quoteTable(t[0], t[1]) : this.quoteTable(t, true);
+    mainTable.quotedName = !Array.isArray(mainTable.name) ? this.quoteTable(mainTable.name, { ...options, alias: mainTable.as ?? false }) : tableName.map(t => {
+      return Array.isArray(t) ? this.quoteTable(t[0], { ...options, alias: t[1] }) : this.quoteTable(t, { ...options, alias: true });
     }).join(', ');
 
     const mainModelDefinition = mainTable.model?.modelDefinition;
@@ -1668,7 +1668,7 @@ export class AbstractQueryGenerator extends AbstractQueryGeneratorTypeScript {
 
     return {
       join: include.required ? 'INNER JOIN' : include.right && this.dialect.supports['RIGHT JOIN'] ? 'RIGHT OUTER JOIN' : 'LEFT OUTER JOIN',
-      body: this.quoteTable(tableRight, asRight),
+      body: this.quoteTable(tableRight, { ...topLevelInfo.options, ...include, alias: asRight }),
       condition: joinOn,
       attributes: {
         main: [],
@@ -1835,7 +1835,7 @@ export class AbstractQueryGenerator extends AbstractQueryGeneratorTypeScript {
     }
 
     // Generate a wrapped join so that the through table join can be dependent on the target join
-    joinBody = `( ${this.quoteTable(throughTable, throughAs)} INNER JOIN ${this.quoteTable(include.model.getTableName(), includeAs.internalAs)} ON ${targetJoinOn}`;
+    joinBody = `( ${this.quoteTable(throughTable, { ...topLevelInfo.options, ...include, alias: throughAs })} INNER JOIN ${this.quoteTable(include.model.getTableName(), { ...topLevelInfo.options, ...include, alias: includeAs.internalAs })} ON ${targetJoinOn}`;
     if (throughWhere) {
       joinBody += ` AND ${throughWhere}`;
     }
@@ -2076,18 +2076,10 @@ export class AbstractQueryGenerator extends AbstractQueryGeneratorTypeScript {
     fragment += this._getBeforeSelectAttributesFragment(options);
     fragment += ` ${attributes.join(', ')} FROM ${tables}`;
 
-    if (mainTableAs) {
+    if (options.groupedLimit) {
       fragment += ` AS ${mainTableAs}`;
     }
 
-    if (options.indexHints && this.dialect.supports.indexHints) {
-      for (const hint of options.indexHints) {
-        if (IndexHints[hint.type]) {
-          fragment += ` ${IndexHints[hint.type]} INDEX (${hint.values.map(indexName => this.quoteIdentifiers(indexName)).join(',')})`;
-        }
-      }
-    }
-
     return fragment;
   }
 

packages/core/src/dialects/abstract/query-generator.types.ts

@@ -1,7 +1,8 @@
 import type { Deferrable } from '../../deferrable';
 import type { BaseSqlExpression } from '../../expression-builders/base-sql-expression';
-import type { ReferentialAction } from '../../model';
+import type { IndexHintable, ReferentialAction } from '../../model';
 import type { BindOrReplacements } from '../../sequelize';
+import type { TableHints } from '../../table-hints';
 import type { TableNameOrModel } from './query-generator-typescript';
 import type { ConstraintType } from './query-interface.types';
 import type { WhereOptions } from './where-sql-builder-types';
@@ -101,3 +102,8 @@ export interface AttributeToSqlOptions {
   table: string;
   withoutForeignKeyConstraints?: boolean;
 }
+
+export interface QuoteTableOptions extends IndexHintable {
+  alias: boolean | string;
+  tableHints?: TableHints[];
+}

packages/core/src/dialects/mssql/index.ts

@@ -49,6 +49,7 @@ export class MssqlDialect extends AbstractDialect {
       unquoted: true,
       quoted: false,
     },
+    tableHints: true,
   });
 
   readonly connectionManager: MsSqlConnectionManager;

packages/core/src/dialects/mssql/query-generator.js

@@ -21,7 +21,6 @@ import isPlainObject from 'lodash/isPlainObject';
 import isString from 'lodash/isString';
 
 const DataTypes = require('../../data-types');
-const { TableHints } = require('../../table-hints');
 const { MsSqlQueryGeneratorTypeScript } = require('./query-generator-typescript');
 const randomBytes = require('node:crypto').randomBytes;
 const { Op } = require('../../operators');
@@ -819,23 +818,6 @@ export class MsSqlQueryGenerator extends MsSqlQueryGeneratorTypeScript {
     return 'ROLLBACK TRANSACTION;';
   }
 
-  selectFromTableFragment(options, model, attributes, tables, mainTableAs, where) {
-    this._throwOnEmptyAttributes(attributes, { modelName: model && model.name, as: mainTableAs });
-
-    // mssql overwrite the abstract selectFromTableFragment function.
-    if (options.maxExecutionTimeHintMs != null) {
-      throw new Error(`The maxExecutionTimeMs option is not supported by ${this.dialect.name}`);
-    }
-
-    return joinSQLFragments([
-      'SELECT',
-      attributes.join(', '),
-      `FROM ${tables}`,
-      mainTableAs && `AS ${mainTableAs}`,
-      options.tableHint && TableHints[options.tableHint] && `WITH (${TableHints[options.tableHint]})`,
-    ]);
-  }
-
   addLimitAndOffset(options, model) {
     const offset = options.offset || 0;
     const isSubQuery = options.subQuery === undefined

packages/core/src/model.d.ts

@@ -906,7 +906,7 @@ export interface FindOptions<TAttributes = any>
   /**
    * Use a table hint for the query, only supported in MSSQL.
    */
-  tableHint?: TableHints;
+  tableHints?: TableHints[];
 }
 
 export interface NonNullFindOptions<TAttributes = any> extends FindOptions<TAttributes> {

packages/core/test/unit/dialects/mariadb/query-generator.test.js

@@ -393,53 +393,6 @@ if (dialect === 'mariadb') {
           needsSequelize: true,
         },
       ],
-
-      selectFromTableFragment: [
-        {
-          arguments: [{}, null, ['*'], '`Project`'],
-          expectation: 'SELECT * FROM `Project`',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.USE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` USE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.FORCE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` FORCE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.IGNORE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` IGNORE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.USE, values: ['index_project_on_name', 'index_project_on_name_and_foo'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` USE INDEX (`index_project_on_name`,`index_project_on_name_and_foo`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: 'FOO', values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project`',
-        },
-      ],
     };
 
     each(suites, (tests, suiteTitle) => {

packages/core/test/unit/dialects/mssql/query-generator.test.js

@@ -93,38 +93,6 @@ if (current.dialect.name === 'mssql') {
       });
     });
 
-    it('selectFromTableFragment', function () {
-      // Base case
-      expectsql(this.queryGenerator.selectFromTableFragment({}, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName', 'WHERE id=1'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName',
-      });
-
-      // With tableHint - nolock
-      expectsql(this.queryGenerator.selectFromTableFragment({ tableHint: TableHints.NOLOCK }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName WITH (NOLOCK)',
-      });
-
-      // With tableHint - NOWAIT
-      expectsql(this.queryGenerator.selectFromTableFragment({ tableHint: TableHints.NOWAIT }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName WITH (NOWAIT)',
-      });
-
-      // With limit
-      expectsql(this.queryGenerator.selectFromTableFragment({ limit: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName',
-      });
-
-      // With offset
-      expectsql(this.queryGenerator.selectFromTableFragment({ offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName',
-      });
-
-      // With both limit and offset
-      expectsql(this.queryGenerator.selectFromTableFragment({ limit: 10, offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
-        mssql: 'SELECT id, name FROM myTable AS myOtherName',
-      });
-    });
-
     it('getPrimaryKeyConstraintQuery', function () {
       expectsql(this.queryGenerator.getPrimaryKeyConstraintQuery('myTable', 'myColumnName'), {
         mssql: 'SELECT K.TABLE_NAME AS tableName, K.COLUMN_NAME AS columnName, K.CONSTRAINT_NAME AS constraintName FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS AS C JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS K ON C.TABLE_NAME = K.TABLE_NAME AND C.CONSTRAINT_CATALOG = K.CONSTRAINT_CATALOG AND C.CONSTRAINT_SCHEMA = K.CONSTRAINT_SCHEMA AND C.CONSTRAINT_NAME = K.CONSTRAINT_NAME WHERE C.CONSTRAINT_TYPE = \'PRIMARY KEY\' AND K.COLUMN_NAME = N\'myColumnName\' AND K.TABLE_NAME = N\'myTable\';',

packages/core/test/unit/dialects/mysql/query-generator.test.js

@@ -392,53 +392,6 @@ if (dialect === 'mysql') {
           needsSequelize: true,
         },
       ],
-
-      selectFromTableFragment: [
-        {
-          arguments: [{}, null, ['*'], '`Project`'],
-          expectation: 'SELECT * FROM `Project`',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.USE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` USE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.FORCE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` FORCE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.IGNORE, values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` IGNORE INDEX (`index_project_on_name`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: IndexHints.USE, values: ['index_project_on_name', 'index_project_on_name_and_foo'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project` USE INDEX (`index_project_on_name`,`index_project_on_name_and_foo`)',
-        }, {
-          arguments: [
-            { indexHints: [{ type: 'FOO', values: ['index_project_on_name'] }] },
-            null,
-            ['*'],
-            '`Project`',
-          ],
-          expectation: 'SELECT * FROM `Project`',
-        },
-      ],
     };
 
     each(suites, (tests, suiteTitle) => {