From e6e20efb2c9e5059c58cec8e93c5d87abc93f576 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Sat, 6 Nov 2021 14:30:58 +0100 Subject: [PATCH] Allow skip binding in limit and offset #4805 --- .../mssql/query/mssql-querycompiler.js | 20 ++------- .../mysql/query/mysql-querycompiler.js | 6 +-- .../oracle/query/oracle-querycompiler.js | 13 +++--- .../sqlite3/query/sqlite-querycompiler.js | 7 +-- lib/query/querybuilder.js | 16 ++++++- lib/query/querycompiler.js | 25 ++++++----- test/unit/query/builder.js | 44 +++++++++++++++---- types/index.d.ts | 4 +- 8 files changed, 80 insertions(+), 55 deletions(-) diff --git a/lib/dialects/mssql/query/mssql-querycompiler.js b/lib/dialects/mssql/query/mssql-querycompiler.js index b323714be8..e8a4520856 100644 --- a/lib/dialects/mssql/query/mssql-querycompiler.js +++ b/lib/dialects/mssql/query/mssql-querycompiler.js @@ -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() { @@ -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; diff --git a/lib/dialects/mysql/query/mysql-querycompiler.js b/lib/dialects/mysql/query/mysql-querycompiler.js index 313f51910e..382f2f7c16 100644 --- a/lib/dialects/mysql/query/mysql-querycompiler.js +++ b/lib/dialects/mysql/query/mysql-querycompiler.js @@ -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}`; } diff --git a/lib/dialects/oracle/query/oracle-querycompiler.js b/lib/dialects/oracle/query/oracle-querycompiler.js index 0a1c979236..032281e783 100644 --- a/lib/dialects/oracle/query/oracle-querycompiler.js +++ b/lib/dialects/oracle/query/oracle-querycompiler.js @@ -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 )}`; } @@ -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) ); } } diff --git a/lib/dialects/sqlite3/query/sqlite-querycompiler.js b/lib/dialects/sqlite3/query/sqlite-querycompiler.js index 39c0f934e3..44d3931b24 100644 --- a/lib/dialects/sqlite3/query/sqlite-querycompiler.js +++ b/lib/dialects/sqlite3/query/sqlite-querycompiler.js @@ -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')}`; } } diff --git a/lib/query/querybuilder.js b/lib/query/querybuilder.js index 50c76c1a82..a4c982484a 100644 --- a/lib/query/querybuilder.js +++ b/lib/query/querybuilder.js @@ -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; @@ -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; } diff --git a/lib/query/querycompiler.js b/lib/query/querycompiler.js index 6e9084485e..0ebdbc8178 100644 --- a/lib/query/querycompiler.js +++ b/lib/query/querycompiler.js @@ -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. diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js index 9c32f063df..0d04394be3 100644 --- a/test/unit/query/builder.js +++ b/test/unit/query/builder.js @@ -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 = { @@ -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 }; @@ -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('*') @@ -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() diff --git a/types/index.d.ts b/types/index.d.ts index 9c77a5f9dd..e0ef6d2a0c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -605,8 +605,8 @@ export declare namespace Knex { clear(statement: ClearStatements): QueryBuilder; // Paging - offset(offset: number): QueryBuilder; - limit(limit: number): QueryBuilder; + offset(offset: number, options?: boolean | Readonly<{skipBinding?: boolean}>): QueryBuilder; + limit(limit: number, options?: string | Readonly<{skipBinding?: boolean}>): QueryBuilder; // Aggregation count: AsymmetricAggregation>;