diff --git a/lib/query/builder.js b/lib/query/builder.js index 27546f15a3..ed645e97df 100644 --- a/lib/query/builder.js +++ b/lib/query/builder.js @@ -14,6 +14,7 @@ const { isBoolean, isEmpty, isFunction, + isNil, isNumber, isObject, isString, @@ -856,7 +857,17 @@ assign(Builder.prototype, { // Only allow a single "offset" to be set for the current query. offset(value) { - this._single.offset = value; + if (isNil(value) || value instanceof Raw || value instanceof Builder) { + // Builder for backward compatibility + this._single.offset = value; + } else { + const val = parseInt(value, 10); + if (isNaN(val)) { + this.client.logger.warn('A valid integer must be provided to offset'); + } else { + this._single.offset = val; + } + } return this; }, diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js index 78a1b1a376..eba8f0867d 100644 --- a/test/unit/query/builder.js +++ b/test/unit/query/builder.js @@ -40,6 +40,35 @@ const clientsWithNullAsDefault = { ), }; +const customLoggerConfig = { + log: { + warn: function(message) { + throw new Error(message); + }, + }, +}; +const clientsWithCustomLoggerForTestWarnings = { + mysql: new MySQL_Client( + Object.assign({ client: 'mysql' }, customLoggerConfig) + ), + pg: new PG_Client(Object.assign({ client: 'pg' }, customLoggerConfig)), + 'pg-redshift': new Redshift_Client( + Object.assign({ client: 'redshift' }, customLoggerConfig) + ), + oracledb: new Oracledb_Client( + Object.assign({ client: 'oracledb' }, customLoggerConfig) + ), + sqlite3: new SQLite3_Client( + Object.assign( + { client: 'sqlite3' }, + { ...customLoggerConfig, ...useNullAsDefaultConfig } + ) + ), + mssql: new MSSQL_Client( + Object.assign({ client: 'mssql' }, customLoggerConfig) + ), +}; + // note: as a workaround, we are using postgres here, since that's using the default " field wrapping // otherwise subquery cloning would need to be fixed. See: https://github.com/tgriesser/knex/pull/2063 function qb() { @@ -3868,6 +3897,39 @@ describe('QueryBuilder', () => { ); }); + it('limits and offsets with raw', () => { + testsql( + qb() + .select('*') + .from('users') + .offset(raw('5')) + .limit(raw('10')), + { + mysql: { + sql: 'select * from `users` limit ? offset 5', + bindings: [10], + }, + mssql: { + sql: 'select * from [users] offset 5 rows fetch next ? rows only', + bindings: [10], + }, + oracledb: { + sql: + 'select * from (select row_.*, ROWNUM rownum_ from (select * from "users") row_ where rownum <= ?) where rownum_ > 5', + bindings: [15], + }, + pg: { + sql: 'select * from "users" limit ? offset 5', + bindings: [10], + }, + 'pg-redshift': { + sql: 'select * from "users" limit ? offset 5', + bindings: [10], + }, + } + ); + }); + it('limits and raw selects', () => { testsql( qb() @@ -7177,6 +7239,100 @@ describe('QueryBuilder', () => { ); }); + it('should throw warning with null call in limit', function() { + try { + testsql( + qb() + .from('test') + .limit(null), + { + mysql: { + sql: 'select * from `test`', + bindings: [], + }, + mssql: { + sql: 'select * from [test]', + bindings: [], + }, + pg: { + sql: 'select * from "test"', + bindings: [], + }, + 'pg-redshift': { + sql: 'select * from "test"', + bindings: [], + }, + }, + clientsWithCustomLoggerForTestWarnings + ); + } catch (error) { + expect(error.message).to.equal( + 'A valid integer must be provided to limit' + ); + } + }); + + it('should do nothing with offset when passing null', () => { + testsql( + qb() + .from('test') + .limit(10) + .offset(null), + { + mysql: { + sql: 'select * from `test` limit ?', + bindings: [10], + }, + mssql: { + sql: 'select top (?) * from [test]', + bindings: [10], + }, + pg: { + sql: 'select * from "test" limit ?', + bindings: [10], + }, + 'pg-redshift': { + sql: 'select * from "test" limit ?', + bindings: [10], + }, + } + ); + }); + + it('should throw warning with wrong value call in offset', function() { + try { + testsql( + qb() + .from('test') + .limit(10) + .offset('$10'), + { + mysql: { + sql: 'select * from `test` limit ?', + bindings: [10], + }, + mssql: { + sql: 'select top (?) * from [test]', + bindings: [10], + }, + pg: { + sql: 'select * from "test" limit ?', + bindings: [10], + }, + 'pg-redshift': { + sql: 'select * from "test" limit ?', + bindings: [10], + }, + }, + clientsWithCustomLoggerForTestWarnings + ); + } catch (error) { + expect(error.message).to.equal( + 'A valid integer must be provided to offset' + ); + } + }); + it('should clear offset when passing null', () => { testsql( qb()