Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why does limit(0) omit LIMIT from the final query? #845

Closed
camhashemi opened this issue Mar 24, 2020 · 4 comments
Closed

why does limit(0) omit LIMIT from the final query? #845

camhashemi opened this issue Mar 24, 2020 · 4 comments

Comments

@camhashemi
Copy link

source code

this means that providing limit(0) is equivalent to not calling limit at all, which is odd behavior imho.

At least for MySQL, limit 0 doesn't behave this way.

@camhashemi camhashemi changed the title why does limit(0) omit LIMIT 0 from the final query? why does limit(0) omit LIMIT from the final query? Mar 24, 2020
@bastman
Copy link

bastman commented Apr 6, 2020

I agree with @camhashemi .

I find silently ignoring the "limit" clause weird, if limit is set to a value <1.
I'd favour - to let the underlying db handle this case (e.g. fail).

Currently passing invalid arguments (limit <1) will silently expose all db rows.
This may lead to hard to spot bugs.
I'd prefer the fail-fast / in-your-face approach.

imho, limit=null would reflect the intention to get all rows without any limit.

What do you think?

@camhashemi
Copy link
Author

especially with the query builder style, the less custom interpretation between DSL and SQL the better.

passing null is one way to signal "no limit", the other way would be to omit the call to limit at all.

@xirc
Copy link

xirc commented May 7, 2020

I also agree with @camhashemi @bastman .

I found the behavior when I have written some test cases for my app.
The behavior is not good imho, even if I should write tests.

Here is a sample project.
https://github.com/xirc/exposed-limit-example

@Tapac
Copy link
Contributor

Tapac commented May 7, 2020

Fixed. Will be available in 0.24.2

@Tapac Tapac closed this as completed May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants