diff --git a/lib/dialects/mssql/query/compiler.js b/lib/dialects/mssql/query/compiler.js index 10a7179ec7..553bca4558 100644 --- a/lib/dialects/mssql/query/compiler.js +++ b/lib/dialects/mssql/query/compiler.js @@ -262,6 +262,7 @@ class QueryCompiler_MSSQL extends QueryCompiler { let distinctClause = ''; if (this.onlyUnions()) return ''; const top = this.top(); + const hints = this._hintComments() const columns = this.grouped.columns || []; let i = -1, sql = []; @@ -285,7 +286,7 @@ class QueryCompiler_MSSQL extends QueryCompiler { if (sql.length === 0) sql = ['*']; return ( - `select ${distinctClause}` + + `select ${hints}${distinctClause}` + (top ? top + ' ' : '') + sql.join(', ') + (this.tableName ? ` from ${this.tableName}` : '') diff --git a/lib/query/builder.js b/lib/query/builder.js index 3f9b768b67..332f361219 100644 --- a/lib/query/builder.js +++ b/lib/query/builder.js @@ -185,6 +185,25 @@ assign(Builder.prototype, { return this; }, + // Adds a single hint or an array of hits to the list of "hintComments" on the query. + hintComment(hints) { + hints = Array.isArray(hints) ? hints : [hints] + if (hints.some((hint) => !isString(hint))) { + throw new Error('Hint comment must be a string'); + } + if (hints.some((hint) => hint.includes('/*') || hint.includes('*/'))) { + throw new Error('Hint comment cannot include "/*" or "*/"'); + } + if (hints.some((hint) => hint.includes('?'))) { + throw new Error('Hint comment cannot include "?"'); + } + this._statements.push({ + grouping: 'hintComments', + value: hints, + }) + return this; + }, + // Prepends the `schemaName` on `tableName` defined by `.table` and `.join`. withSchema(schemaName) { this._single.schema = schemaName; @@ -1064,7 +1083,7 @@ assign(Builder.prototype, { // Remove everything from statement clause clear(statement) { - if (!['with', 'select', 'columns', 'where', 'union', 'join', 'group', 'order', 'having', 'limit', 'offset', 'counter', 'counters'].includes(statement)) + if (!['with', 'select', 'columns', 'hintComments', 'where', 'union', 'join', 'group', 'order', 'having', 'limit', 'offset', 'counter', 'counters'].includes(statement)) throw new Error(`Knex Error: unknown statement '${statement}'`); if (statement.startsWith('counter')) return this.clearCounters(); if (statement === 'select') { statement = 'columns'; } diff --git a/lib/query/compiler.js b/lib/query/compiler.js index dd2faa14e4..28d4173544 100644 --- a/lib/query/compiler.js +++ b/lib/query/compiler.js @@ -182,10 +182,18 @@ class QueryCompiler { ); } + _hintComments() { + let hints = this.grouped.hintComments || []; + hints = hints.map((hint) => compact(hint.value).join(' ')); + hints = compact(hints).join(' '); + return hints ? `/*+ ${hints} */ ` : '' + } + // Compiles the columns in the query, specifying if an item was distinct. columns() { let distinctClause = ''; if (this.onlyUnions()) return ''; + const hints = this._hintComments() const columns = this.grouped.columns || []; let i = -1, sql = []; @@ -208,7 +216,7 @@ class QueryCompiler { } if (sql.length === 0) sql = ['*']; return ( - `select ${distinctClause}` + + `select ${hints}${distinctClause}` + sql.join(', ') + (this.tableName ? ` from ${this.single.only ? 'only ' : ''}${this.tableName}` diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js index f93e6df352..dc987371c5 100644 --- a/test/integration/builder/selects.js +++ b/test/integration/builder/selects.js @@ -150,6 +150,78 @@ module.exports = function (knex) { }); }); + it('#4199 - adheres to hint comments', async function () { + const expectedErrors = { + mysql: { + code: 'ER_QUERY_TIMEOUT', + errno: 3024, + sqlMessage: 'Query execution was interrupted, maximum statement execution time exceeded', + }, + mysql2: { + errno: 3024, + sqlMessage: 'Query execution was interrupted, maximum statement execution time exceeded', + }, + } + if (!expectedErrors[knex.client.driverName]) { + return this.skip(); + } + const baseQuery = knex('accounts') + .select('id', knex.raw('sleep(0.1)')) + .limit(2) + await expect( + baseQuery.clone() + ).to.eventually.be.fulfilled.and.to.have.lengthOf(2) + await expect( + baseQuery.clone().hintComment('max_execution_time(10)') + ).to.eventually.be.rejected.and.to.deep.include(expectedErrors[knex.client.driverName]) + }); + + it('#4199 - ignores invalid hint comments', async function () { + return knex + .select('id') + .orderBy('id') + .from('accounts') + .hintComment('invalid()') + .testSql(function (tester) { + tester( + 'mysql', + 'select /*+ invalid() */ `id` from `accounts` order by `id` asc', + [], + [{id: 1}, {id: 2}, {id: 3}, {id: 4}, {id: 5}, {id: 7}] + ); + tester( + 'pg', + 'select /*+ invalid() */ "id" from "accounts" order by "id" asc', + [], + [{id: '1'}, {id: '2'}, {id: '3'}, {id: '4'}, {id: '5'}, {id: '7'}] + ); + tester( + 'pg-redshift', + 'select /*+ invalid() */ "id" from "accounts" order by "id" asc', + [], + [{id: '1'}, {id: '2'}, {id: '3'}, {id: '4'}, {id: '5'}, {id: '6'}] + ); + tester( + 'sqlite3', + 'select /*+ invalid() */ `id` from `accounts` order by `id` asc', + [], + [{id: 1}, {id: 2}, {id: 3}, {id: 4}, {id: 5}, {id: 6}] + ); + tester( + 'oracledb', + 'select /*+ invalid() */ "id" from "accounts" order by "id" asc', + [], + [{id: 1}, {id: 2}, {id: 3}, {id: 4}, {id: 5}, {id: 7}] + ); + tester( + 'mssql', + 'select /*+ invalid() */ [id] from [accounts] order by [id] asc', + [], + [{id: '1'}, {id: '2'}, {id: '3'}, {id: '4'}, {id: '5'}, {id: '7'}] + ); + }); + }); + it('returns a single entry with first', function () { return knex .first('id', 'first_name') diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js index 798da0f92c..b9a8add55d 100644 --- a/test/unit/query/builder.js +++ b/test/unit/query/builder.js @@ -752,6 +752,7 @@ describe('QueryBuilder', () => { }) .join('tableJoin','id', 'id') .select(['id']) + .hintComment('hint()') .where('id', '<', 10) .groupBy('id') .groupBy('id', 'desc') @@ -770,6 +771,7 @@ describe('QueryBuilder', () => { .clear('columns') .select(['id']) .clear('select') + .clear('hintComments') .clear('where') .clear('group') .clear('order') @@ -8324,6 +8326,163 @@ describe('QueryBuilder', () => { }); }); + it('#4199 - allows an hint comment', () => { + testsql( + qb() + .from('testtable') + .hintComment('hint()'), + { + mysql: { + sql: 'select /*+ hint() */ * from `testtable`', + bindings: [], + }, + oracledb: { + sql: 'select /*+ hint() */ * from "testtable"', + bindings: [], + }, + mssql: { + sql: 'select /*+ hint() */ * from [testtable]', + bindings: [], + }, + pg: { + sql: 'select /*+ hint() */ * from "testtable"', + bindings: [], + }, + 'pg-redshift': { + sql: 'select /*+ hint() */ * from "testtable"', + bindings: [], + }, + }, + ) + }); + + it('#4199 - allows multiple hint comments', () => { + testsql( + qb() + .from('testtable') + .hintComment(['hint1()', 'hint2()']) + .hintComment('hint3()'), + { + mysql: { + sql: 'select /*+ hint1() hint2() hint3() */ * from `testtable`', + bindings: [], + }, + oracledb: { + sql: 'select /*+ hint1() hint2() hint3() */ * from "testtable"', + bindings: [], + }, + mssql: { + sql: 'select /*+ hint1() hint2() hint3() */ * from [testtable]', + bindings: [], + }, + pg: { + sql: 'select /*+ hint1() hint2() hint3() */ * from "testtable"', + bindings: [], + }, + 'pg-redshift': { + sql: 'select /*+ hint1() hint2() hint3() */ * from "testtable"', + bindings: [], + }, + }, + ) + }); + + it('#4199 - allows hint comments in subqueries', () => { + testsql( + qb() + .select({ + c1: 'c1', + c2: qb().select('c2').from('t2').hintComment('hint2()').limit(1), + }) + .from('t1') + .hintComment('hint1()'), + { + mysql: { + sql: 'select /*+ hint1() */ `c1` as `c1`, (select /*+ hint2() */ `c2` from `t2` limit ?) as `c2` from `t1`', + bindings: [1], + }, + oracledb: { + sql: 'select /*+ hint1() */ "c1" "c1", (select * from (select /*+ hint2() */ "c2" from "t2") where rownum <= ?) "c2" from "t1"', + bindings: [1], + }, + mssql: { + sql: 'select /*+ hint1() */ [c1] as [c1], (select /*+ hint2() */ top (?) [c2] from [t2]) as [c2] from [t1]', + bindings: [1], + }, + pg: { + sql: 'select /*+ hint1() */ "c1" as "c1", (select /*+ hint2() */ "c2" from "t2" limit ?) as "c2" from "t1"', + bindings: [1], + }, + 'pg-redshift': { + sql: 'select /*+ hint1() */ "c1" as "c1", (select /*+ hint2() */ "c2" from "t2" limit ?) as "c2" from "t1"', + bindings: [1], + }, + }, + ) + }); + + it('#4199 - allows hint comments in unions', () => { + testsql( + qb() + .from('t1') + .hintComment('hint1()') + .unionAll(qb().from('t2').hintComment('hint2()')), + { + mysql: { + sql: 'select /*+ hint1() */ * from `t1` union all select /*+ hint2() */ * from `t2`', + bindings: [], + }, + oracledb: { + sql: 'select /*+ hint1() */ * from "t1" union all select /*+ hint2() */ * from "t2"', + bindings: [], + }, + mssql: { + sql: 'select /*+ hint1() */ * from [t1] union all select /*+ hint2() */ * from [t2]', + bindings: [], + }, + pg: { + sql: 'select /*+ hint1() */ * from "t1" union all select /*+ hint2() */ * from "t2"', + bindings: [], + }, + 'pg-redshift': { + sql: 'select /*+ hint1() */ * from "t1" union all select /*+ hint2() */ * from "t2"', + bindings: [], + }, + }, + ) + }); + + it('#4199 - forbids "/*", "*/" and "?" in hint comments', () => { + expect(() => { + qb().from('testtable').hintComment('hint() /*').toString(); + }).to.throw( + 'Hint comment cannot include "/*" or "*/"' + ); + expect(() => { + qb().from('testtable').hintComment('hint() */').toString(); + }).to.throw( + 'Hint comment cannot include "/*" or "*/"' + ); + expect(() => { + qb().from('testtable').hintComment('hint(?)').toString(); + }).to.throw( + 'Hint comment cannot include "?"' + ); + }); + + it('#4199 - forbids non-strings as hint comments', () => { + expect(() => { + qb().from('testtable').hintComment(47).toString(); + }).to.throw( + 'Hint comment must be a string' + ); + expect(() => { + qb().from('testtable').hintComment(raw('hint(?)', [47])).toString(); + }).to.throw( + 'Hint comment must be a string' + ); + }); + it('Any undefined binding in a SELECT query should throw an error', () => { const qbuilders = [ { diff --git a/types/index.d.ts b/types/index.d.ts index f672b7ed24..2828ea9aea 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -460,13 +460,14 @@ declare namespace Knex { // // QueryInterface // - type ClearStatements = "with" | "select" | "columns" | "where" | "union" | "join" | "group" | "order" | "having" | "limit" | "offset" | "counter" | "counters"; + type ClearStatements = "with" | "select" | "columns" | "hintComments" | "where" | "union" | "join" | "group" | "order" | "having" | "limit" | "offset" | "counter" | "counters"; interface QueryInterface { select: Select; as: As; columns: Select; column: Select; + hintComment: HintComment; from: Table; into: Table; table: Table; @@ -978,6 +979,11 @@ declare namespace Knex { ): QueryBuilder; } + interface HintComment { + (hint: string): QueryBuilder; + (hints: readonly string[]): QueryBuilder; + } + interface Table { < TTable extends TableNames,