Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow skip binding in limit and offset #4805 #4811

Merged
merged 1 commit into from Nov 8, 2021
Merged

Allow skip binding in limit and offset #4805 #4811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 4 additions & 16 deletions lib/dialects/mssql/query/mssql-querycompiler.js
View file
Open in desktop
Expand Up @@ -502,11 +502,7 @@ class QueryCompiler_MSSQL extends QueryCompiler {
const noLimit = !this.single.limit && this.single.limit !== 0;
const noOffset = !this.single.offset;
if (noLimit || !noOffset) return '';
return `top (${this.client.parameter(
this.single.limit,
this.builder,
this.bindingsHolder
)})`;
return `top (${this._getValueOrParameterFromAttribute('limit')})`;
}

limit() {
Expand All @@ -518,19 +514,11 @@ class QueryCompiler_MSSQL extends QueryCompiler {
const noOffset = !this.single.offset;
if (noOffset) return '';
let offset = `offset ${
noOffset
? '0'
: this.client.parameter(
this.single.offset,
this.builder,
this.bindingsHolder
)
noOffset ? '0' : this._getValueOrParameterFromAttribute('offset')
} rows`;
if (!noLimit) {
offset += ` fetch next ${this.client.parameter(
this.single.limit,
this.builder,
this.bindingsHolder
offset += ` fetch next ${this._getValueOrParameterFromAttribute(
'limit'
)} rows only`;
}
return offset;
Expand Down
6 changes: 1 addition & 5 deletions lib/dialects/mysql/query/mysql-querycompiler.js
View file
Open in desktop
Expand Up @@ -146,11 +146,7 @@ class QueryCompiler_MySQL extends QueryCompiler {
const limit =
this.single.offset && noLimit
? '18446744073709551615'
: this.client.parameter(
this.single.limit,
this.builder,
this.bindingsHolder
);
: this._getValueOrParameterFromAttribute('limit');
return `limit ${limit}`;
}

Expand Down
13 changes: 7 additions & 6 deletions lib/dialects/oracle/query/oracle-querycompiler.js
View file
Open in desktop
Expand Up @@ -315,10 +315,9 @@ class QueryCompiler_Oracle extends QueryCompiler {
query = query || '';

if (hasLimit && !offset) {
return `select * from (${query}) where rownum <= ${this.client.parameter(
limit,
this.builder,
this.bindingsHolder
return `select * from (${query}) where rownum <= ${this._getValueOrParameterFromAttribute(
'limit',
limit
)}`;
}

Expand All @@ -330,10 +329,12 @@ class QueryCompiler_Oracle extends QueryCompiler {
query +
') row_ ' +
'where rownum <= ' +
this.client.parameter(endRow, this.builder, this.bindingsHolder) +
(this.single.skipBinding['offset']
? endRow
: this.client.parameter(endRow, this.builder, this.bindingsHolder)) +
') ' +
'where rownum_ > ' +
this.client.parameter(offset, this.builder, this.bindingsHolder)
this._getValueOrParameterFromAttribute('offset', offset)
);
}
}
Expand Down
7 changes: 2 additions & 5 deletions lib/dialects/sqlite3/query/sqlite-querycompiler.js
View file
Open in desktop
Expand Up @@ -241,11 +241,8 @@ class QueryCompiler_SQLite3 extends QueryCompiler {

// Workaround for offset only,
// see http://stackoverflow.com/questions/10491492/sqllite-with-skip-offset-only-not-limit
return `limit ${this.client.parameter(
noLimit ? -1 : this.single.limit,
this.builder,
this.bindingsHolder
)}`;
this.single.limit = noLimit ? -1 : this.single.limit;
return `limit ${this._getValueOrParameterFromAttribute('limit')}`;
}
}

Expand Down
16 changes: 14 additions & 2 deletions lib/query/querybuilder.js
View file
Open in desktop
Expand Up @@ -943,8 +943,18 @@ class Builder extends EventEmitter {
return this._bool('or').havingRaw(sql, bindings);
}

// set the skip binding parameter (= insert the raw value in the query) for an attribute.
_setSkipBinding(attribute, options) {
let skipBinding = options;
if (isObject(options)) {
skipBinding = options.skipBinding;
}
this._single.skipBinding = this._single.skipBinding || {};
this._single.skipBinding[attribute] = skipBinding;
}

// Only allow a single "offset" to be set for the current query.
offset(value) {
offset(value, options) {
if (value == null || value.isRawInstance || value instanceof Builder) {
// Builder for backward compatibility
this._single.offset = value;
Expand All @@ -958,16 +968,18 @@ class Builder extends EventEmitter {
this._single.offset = val;
}
}
this._setSkipBinding('offset', options);
return this;
}

// Only allow a single "limit" to be set for the current query.
limit(value) {
limit(value, options) {
const val = parseInt(value, 10);
if (isNaN(val)) {
this.client.logger.warn('A valid integer must be provided to limit');
} else {
this._single.limit = val;
this._setSkipBinding('limit', options);
}
return this;
}
Expand Down
25 changes: 15 additions & 10 deletions lib/query/querycompiler.js
View file
Open in desktop
Expand Up @@ -705,23 +705,28 @@ class QueryCompiler {
return !this.grouped.columns && this.grouped.union && !this.tableName;
}

_getValueOrParameterFromAttribute(attribute, rawValue) {
if (this.single.skipBinding[attribute] === true) {
return rawValue !== undefined && rawValue !== null
? rawValue
: this.single[attribute];
}
return this.client.parameter(
this.single[attribute],
this.builder,
this.bindingsHolder
);
}

limit() {
const noLimit = !this.single.limit && this.single.limit !== 0;
if (noLimit) return '';
return `limit ${this.client.parameter(
this.single.limit,
this.builder,
this.bindingsHolder
)}`;
return `limit ${this._getValueOrParameterFromAttribute('limit')}`;
}

offset() {
if (!this.single.offset) return '';
return `offset ${this.client.parameter(
this.single.offset,
this.builder,
this.bindingsHolder
)}`;
return `offset ${this._getValueOrParameterFromAttribute('offset')}`;
}

// Compiles a `delete` query.
Expand Down
44 changes: 35 additions & 9 deletions test/unit/query/builder.js
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ const Redshift_Client = require('../../../lib/dialects/redshift');
const Oracledb_Client = require('../../../lib/dialects/oracledb');
const SQLite3_Client = require('../../../lib/dialects/sqlite3');
const MSSQL_Client = require('../../../lib/dialects/mssql');
const CockroachDB_Client = require('../../../lib/dialects/cockroachdb');

// use driverName as key
const clients = {
Expand All @@ -17,6 +18,7 @@ const clients = {
oracledb: new Oracledb_Client({ client: 'oracledb' }),
sqlite3: new SQLite3_Client({ client: 'sqlite3' }),
mssql: new MSSQL_Client({ client: 'mssql' }),
cockroachdb: new CockroachDB_Client({ client: 'cockroachdb' }),
};

const useNullAsDefaultConfig = { useNullAsDefault: true };
Expand Down Expand Up @@ -2108,15 +2110,6 @@ describe('QueryBuilder', () => {
});
});

// it("handles grouped mysql unions", function() {
// chain = myqb().union(
// raw(myqb().select('*').from('users').where('id', '=', 1)).wrap('(', ')'),
// raw(myqb().select('*').from('users').where('id', '=', 2)).wrap('(', ')')
// ).orderBy('id').limit(10).toSQL();
// expect(chain.sql).to.equal('(select * from `users` where `id` = ?) union (select * from `users` where `id` = ?) order by `id` asc limit ?');
// expect(chain.bindings).to.eql([1, 2, 10]);
// });

it('union alls', () => {
const chain = qb()
.select('*')
Expand Down Expand Up @@ -3586,6 +3579,39 @@ describe('QueryBuilder', () => {
});
});

it('limits with skip binding', () => {
testsql(
qb()
.select('*')
.from('users')
.limit(10, { skipBinding: true })
.offset(5, true),
{
mysql: {
sql: 'select * from `users` limit 10 offset 5',
},
sqlite3: {
sql: 'select * from `users` limit 10 offset 5',
},
mssql: {
sql: 'select * from [users] offset 5 rows fetch next 10 rows only',
},
oracledb: {
sql: 'select * from (select row_.*, ROWNUM rownum_ from (select * from "users") row_ where rownum <= 15) where rownum_ > 5',
},
pg: {
sql: 'select * from "users" limit 10 offset 5',
},
cockroachdb: {
sql: 'select * from "users" limit 10 offset 5',
},
'pg-redshift': {
sql: 'select * from "users" limit 10 offset 5',
},
}
);
});

it('limits and raw selects', () => {
testsql(
qb()
Expand Down
4 changes: 2 additions & 2 deletions types/index.d.ts
View file
Open in desktop
Expand Up @@ -605,8 +605,8 @@ export declare namespace Knex {
clear(statement: ClearStatements): QueryBuilder<TRecord, TResult>;

// Paging
offset(offset: number): QueryBuilder<TRecord, TResult>;
limit(limit: number): QueryBuilder<TRecord, TResult>;
offset(offset: number, options?: boolean | Readonly<{skipBinding?: boolean}>): QueryBuilder<TRecord, TResult>;
limit(limit: number, options?: string | Readonly<{skipBinding?: boolean}>): QueryBuilder<TRecord, TResult>;

// Aggregation
count: AsymmetricAggregation<TRecord, TResult, Lookup<ResultTypes.Registry, "Count", number | string>>;
Expand Down