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

Got "LIMIT must not be negative" error as long as it is not a literal #9506

Closed
SteveLauC opened this issue Mar 8, 2024 · 4 comments · Fixed by #9790
Closed

Got "LIMIT must not be negative" error as long as it is not a literal #9506

SteveLauC opened this issue Mar 8, 2024 · 4 comments · Fixed by #9790
Labels
bug Something isn't working

Comments

@SteveLauC
Copy link
Contributor

SteveLauC commented Mar 8, 2024

Describe the bug

$ datafusion-cli --color
DataFusion CLI v36.0.0
❯ select * from '1.parquet' limit 1 + 1;
Error during planning: LIMIT must not be negative
❯

It should work as long as it can be evaluated without schema or data, here is how duckdb handles it:

$ duckdb
v0.10.0 20b1486d11
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
D create table foo (id int);
D select * from foo limit 1 + 1;
┌────────┐
│   id   │
│ int32  │
├────────┤
│ 0 rows │
└────────┘

To Reproduce

See above, the table (parquet file) should not matter.

Expected behavior

1 + 1 should be evaluated to a constant 2.

Additional context

Related code:

https://github.com/apache/arrow-datafusion/blob/fc81bf10ea7cde55c6d58a670e15bfd0581ec8c2/datafusion/sql/src/query.rs#L240-L257

@SteveLauC SteveLauC added the bug Something isn't working label Mar 8, 2024
@SteveLauC SteveLauC changed the title Got LIMIT must not be negative error as long as it is not a literal Got "LIMIT must not be negative" error as long as it is not a literal Mar 8, 2024
@SteveLauC
Copy link
Contributor Author

So if DataFusion wants to follow DuckDB's behavior, then I guess we should evaluate that SQLExpr rather than simply converting it to DataFusion's Expr type.

@jonahgao
Copy link
Member

I think ideally it should support any scalar expression.
In PostgreSQL, even functions and scalar subqueries are allowed.

psql=> select * from t limit (select * from t limit 1);
 a
---
 1
(1 row)

psql=> select * from t limit random() + 1;
 a
---
 1
(1 row)

@SteveLauC
Copy link
Contributor Author

any scalar expression

Yeah, as long as it can be evaluated to a number, it should work.

@alamb
Copy link
Contributor

alamb commented Mar 24, 2024

I think we could run the Expr simplifier / const folder on the limit value

I am not sure we can do so from within the planner itself 🤔

https://github.com/apache/arrow-datafusion/blob/1e4ddb6d86328cb6596bb50da9ccc654f19a83ea/datafusion/sql/src/query.rs#L249C1-L253C1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants