Skip to content

Commit

Permalink
Rename method to hintComment
Browse files Browse the repository at this point in the history
  • Loading branch information
martinmacko47 committed Jan 25, 2021
1 parent 65f488b commit ad8b9a6
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 43 deletions.
2 changes: 1 addition & 1 deletion lib/dialects/mssql/query/mssql-querycompiler.js
Expand Up @@ -277,7 +277,7 @@ class QueryCompiler_MSSQL extends QueryCompiler {
let distinctClause = '';
if (this.onlyUnions()) return '';
const top = this.top();
const hints = this._optimizerHints()
const hints = this._hintComments()
const columns = this.grouped.columns || [];
let i = -1,
sql = [];
Expand Down
14 changes: 7 additions & 7 deletions lib/query/querybuilder.js
Expand Up @@ -148,20 +148,20 @@ class Builder extends EventEmitter {
return this;
}

// Adds a single hint or an array of hits to the list of "optimizerHints" on the query.
optimizerHint(hints) {
// 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('Optimizer hint must be a string');
throw new Error('Hint comment must be a string');
}
if (hints.some((hint) => hint.includes('/*') || hint.includes('*/'))) {
throw new Error('Optimizer hint cannot include "/*" or "*/"');
throw new Error('Hint comment cannot include "/*" or "*/"');
}
if (hints.some((hint) => hint.includes('?'))) {
throw new Error('Optimizer hint cannot include "?"');
throw new Error('Hint comment cannot include "?"');
}
this._statements.push({
grouping: 'optimizerHints',
grouping: 'hintComments',
value: hints,
})
return this;
Expand Down Expand Up @@ -1059,7 +1059,7 @@ class Builder extends EventEmitter {
'with',
'select',
'columns',
'optimizerHints',
'hintComments',
'where',
'union',
'join',
Expand Down
6 changes: 3 additions & 3 deletions lib/query/querycompiler.js
Expand Up @@ -199,8 +199,8 @@ class QueryCompiler {
);
}

_optimizerHints() {
let hints = this.grouped.optimizerHints || [];
_hintComments() {
let hints = this.grouped.hintComments || [];
hints = hints.map((hint) => compact(hint.value).join(' '));
hints = compact(hints).join(' ');
return hints ? `/*+ ${hints} */ ` : ''
Expand All @@ -210,7 +210,7 @@ class QueryCompiler {
columns() {
let distinctClause = '';
if (this.onlyUnions()) return '';
const hints = this._optimizerHints()
const hints = this._hintComments()
const columns = this.grouped.columns || [];
let i = -1,
sql = [];
Expand Down
8 changes: 4 additions & 4 deletions test/integration/query/selects.js
Expand Up @@ -150,7 +150,7 @@ module.exports = function (knex) {
});
});

it('#4199 - adheres to optimizer hints', async function () {
it('#4199 - adheres to hint comments', async function () {
const expectedErrors = {
mysql: {
code: 'ER_QUERY_TIMEOUT',
Expand All @@ -172,16 +172,16 @@ module.exports = function (knex) {
baseQuery.clone()
).to.eventually.be.fulfilled.and.to.have.lengthOf(2)
await expect(
baseQuery.clone().optimizerHint('max_execution_time(10)')
baseQuery.clone().hintComment('max_execution_time(10)')
).to.eventually.be.rejected.and.to.deep.include(expectedErrors[knex.client.driverName])
});

it('#4199 - ignores invalid optimizer hints', async function () {
it('#4199 - ignores invalid hint comments', async function () {
return knex
.select('id')
.orderBy('id')
.from('accounts')
.optimizerHint('invalid()')
.hintComment('invalid()')
.testSql(function (tester) {
tester(
'mysql',
Expand Down
50 changes: 25 additions & 25 deletions test/unit/query/builder.js
Expand Up @@ -752,7 +752,7 @@ describe('QueryBuilder', () => {
})
.join('tableJoin', 'id', 'id')
.select(['id'])
.optimizerHint('hint()')
.hintComment('hint()')
.where('id', '<', 10)
.groupBy('id')
.groupBy('id', 'desc')
Expand All @@ -771,7 +771,7 @@ describe('QueryBuilder', () => {
.clear('columns')
.select(['id'])
.clear('select')
.clear('optimizerHints')
.clear('hintComments')
.clear('where')
.clear('group')
.clear('order')
Expand Down Expand Up @@ -9225,11 +9225,11 @@ describe('QueryBuilder', () => {
});
});

it('#4199 - allows an optimizer hint', () => {
it('#4199 - allows an hint comment', () => {
testsql(
qb()
.from('testtable')
.optimizerHint('hint()'),
.hintComment('hint()'),
{
mysql: {
sql: 'select /*+ hint() */ * from `testtable`',
Expand All @@ -9255,12 +9255,12 @@ describe('QueryBuilder', () => {
)
});

it('#4199 - allows multiple optimizer hints', () => {
it('#4199 - allows multiple hint comments', () => {
testsql(
qb()
.from('testtable')
.optimizerHint(['hint1()', 'hint2()'])
.optimizerHint('hint3()'),
.hintComment(['hint1()', 'hint2()'])
.hintComment('hint3()'),
{
mysql: {
sql: 'select /*+ hint1() hint2() hint3() */ * from `testtable`',
Expand All @@ -9286,15 +9286,15 @@ describe('QueryBuilder', () => {
)
});

it('#4199 - allows optimizer hints in subqueries', () => {
it('#4199 - allows hint comments in subqueries', () => {
testsql(
qb()
.select({
c1: 'c1',
c2: qb().select('c2').from('t2').optimizerHint('hint2()').limit(1),
c2: qb().select('c2').from('t2').hintComment('hint2()').limit(1),
})
.from('t1')
.optimizerHint('hint1()'),
.hintComment('hint1()'),
{
mysql: {
sql: 'select /*+ hint1() */ `c1` as `c1`, (select /*+ hint2() */ `c2` from `t2` limit ?) as `c2` from `t1`',
Expand All @@ -9320,12 +9320,12 @@ describe('QueryBuilder', () => {
)
});

it('#4199 - allows optimizer hints in unions', () => {
it('#4199 - allows hint comments in unions', () => {
testsql(
qb()
.from('t1')
.optimizerHint('hint1()')
.unionAll(qb().from('t2').optimizerHint('hint2()')),
.hintComment('hint1()')
.unionAll(qb().from('t2').hintComment('hint2()')),
{
mysql: {
sql: 'select /*+ hint1() */ * from `t1` union all select /*+ hint2() */ * from `t2`',
Expand All @@ -9351,34 +9351,34 @@ describe('QueryBuilder', () => {
)
});

it('#4199 - forbids "/*", "*/" and "?" in optimizer hints', () => {
it('#4199 - forbids "/*", "*/" and "?" in hint comments', () => {
expect(() => {
qb().from('testtable').optimizerHint('hint() /*').toString();
qb().from('testtable').hintComment('hint() /*').toString();
}).to.throw(
'Optimizer hint cannot include "/*" or "*/"'
'Hint comment cannot include "/*" or "*/"'
);
expect(() => {
qb().from('testtable').optimizerHint('hint() */').toString();
qb().from('testtable').hintComment('hint() */').toString();
}).to.throw(
'Optimizer hint cannot include "/*" or "*/"'
'Hint comment cannot include "/*" or "*/"'
);
expect(() => {
qb().from('testtable').optimizerHint('hint(?)').toString();
qb().from('testtable').hintComment('hint(?)').toString();
}).to.throw(
'Optimizer hint cannot include "?"'
'Hint comment cannot include "?"'
);
});

it('#4199 - forbids non-strings as optimizer hints', () => {
it('#4199 - forbids non-strings as hint comments', () => {
expect(() => {
qb().from('testtable').optimizerHint(47).toString();
qb().from('testtable').hintComment(47).toString();
}).to.throw(
'Optimizer hint must be a string'
'Hint comment must be a string'
);
expect(() => {
qb().from('testtable').optimizerHint(raw('hint(?)', [47])).toString();
qb().from('testtable').hintComment(raw('hint(?)', [47])).toString();
}).to.throw(
'Optimizer hint must be a string'
'Hint comment must be a string'
);
});

Expand Down
6 changes: 3 additions & 3 deletions types/index.d.ts
Expand Up @@ -472,14 +472,14 @@ export declare namespace Knex {
//
// QueryInterface
//
type ClearStatements = "with" | "select" | "columns" | "optimizerHints" | "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<TRecord extends {} = any, TResult = any> {
select: Select<TRecord, TResult>;
as: As<TRecord, TResult>;
columns: Select<TRecord, TResult>;
column: Select<TRecord, TResult>;
optimizerHint: OptimizerHint<TRecord, TResult>;
hintComment: HintComment<TRecord, TResult>;
from: Table<TRecord, TResult>;
into: Table<TRecord, TResult>;
table: Table<TRecord, TResult>;
Expand Down Expand Up @@ -1001,7 +1001,7 @@ export declare namespace Knex {
): QueryBuilder<TRecord, TResult2>;
}

interface OptimizerHint<TRecord extends {} = any, TResult extends {} = any> {
interface HintComment<TRecord extends {} = any, TResult extends {} = any> {
(hint: string): QueryBuilder<TRecord, TResult>;
(hints: readonly string[]): QueryBuilder<TRecord, TResult>;
}
Expand Down

0 comments on commit ad8b9a6

Please sign in to comment.