Allow skip binding in limit and offset (#4811)

knex · Nov 8, 2021 · fdad316 · fdad316
1 parent ffd0c10
commit fdad316
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -949,8 +949,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;
@@ -964,16 +974,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
@@ -712,23 +712,28 @@ class QueryCompiler {
     );
   }
 
+  _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
@@ -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('*')
@@ -3616,6 +3609,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
@@ -608,8 +608,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>>;