Preventing negative offset by default

[aghArdeshir]

Environment

Knex version: 0.21.17
Database + version: ~#: mysql --version -> mysql Ver 14.14 Distrib 5.5.62, for debian-linux-gnu (x86_64) using readline 6.3
OS: Debian

Bug

1. I mistakenly set a negative offset:

// ...
page(page: number = 1) {
  const offset = (page - 1) * this.limit;
  this.query.offset(offset);
}

I mistakenly passed 0 to this function, and my query became:
select * from `` order by Time desc limit 20 offset -20

2. Error message
   
   (sorry for the blue marks)

3. Reduced test code:

import Knex from 'knex';

const knexConnection = Knex({
  client: 'mysql',
  connection: {
    host: '', //...
    user: '', //...
    password: '', //...
    database: '', //...
  },
});
const query = knexConnection('test').limit(20).offset(-20);

I believe if no negative offset is allowed in mysql query, the knex query-builder should prevent it. (instead of letting the error happen and generated by mysql)

If it is knex that should implement this, and this is known as fix that needs to be done in knex, I'd love to contribute to code.

[kibertoad]

@Ardeshir81 Considering that error thrown by driver in this case is pretty confusing, I think throwing an explicit error from Knex would be an improvement. PR would be most appreciated.

[aghArdeshir]

That's good news @kibertoad . I'll start working on it. I'll ask your help if I had problem understanding things.

[aghArdeshir]

I'm only familiar with mysql. Is negative offset invalid for all databases?

[aghArdeshir]

I will close this issue as I think it is resolved now @kibertoad