Treat offset and limit input the same way #3183
Conversation
Currently, if you give offset a string, it will throw an error like this: ``` code: 'ER_PARSE_ERROR', errno: 1064, sqlMessage: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near \'\'0\'\' at line 1', sqlState: '42000', index: 0, sql: 'select * from `Alert` inner join `AlertType` on `Alert`.`AlertTypeID` = `AlertType`.`AlertTypeID` order by `AlertDate` desc limit 2 offset \'0\'' } ``` This is problematic, seeing how offset is likely used in pagination using a query in the url, which is usually treated as a string. It's weird to me that it's not like this already since `limit`, a relatively similar method, is implemented this way.
|
Alright, I'll take a look at tests later. Haven't really looked through the source code at all, I just went straight for this. On another note, I found this: #2908 |
Good point. Actually this PR has the same problem that .offset() should allow passing knex.Raw as input too and maybe even query builder (not sure if SQL spec allows subqueries in offset...) So implementation or that PR was not complete and the he closed it for some reason. |
|
Yeah, probably it would be easier to reopen/duplicate that one and take it from there. |
|
@AndreasClausen FYI #2908 has been merged. |
|
Right, thank you |
ghost
closed this
Currently, if you give offset a string, it will throw an error like this:
This is problematic, seeing how offset is likely used in pagination using a query in the url, which is usually treated as a string. It's weird to me that it's not like this already since
limit, a relatively similar method, is implemented this way.