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

panic! 'index out of bounds: the len is 0 but the index is 0 with bad sql query #496

Closed
alamb opened this issue Jun 3, 2021 · 4 comments · Fixed by #505
Closed

panic! 'index out of bounds: the len is 0 but the index is 0 with bad sql query #496

alamb opened this issue Jun 3, 2021 · 4 comments · Fixed by #505
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Jun 3, 2021

Describe the bug
A malformed query generates a panic rather than an error

To Reproduce

echo "true" > /tmp/foo.csv
cargo run -p datafusion-cli  --no-default-features

Then run select count(distinct) from foo; via sql:

> CREATE EXTERNAL TABLE foo(bar boolean)
STORED AS CSV
LOCATION '/tmp/foo.csv';
0 rows in set. Query took 0 seconds.
> select count(distinct) from foo;
thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', datafusion/src/physical_plan/aggregates.rs:116:15
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected behavior
I expect an error about "no column specified for count distinct" rather than a panic

@alamb alamb added bug Something isn't working good first issue Good for newcomers labels Jun 3, 2021
@jgoday
Copy link
Contributor

jgoday commented Jun 3, 2021

Hi @alamb Can I try this ?
Should be enough to check if the coerced args in the aggregation returns no result (in physical_plan/aggregates) ?

let arg = coerce(args, input_schema, &signature(fun))?;
if arg.len() == 0 {
    return Err(DataFusionError::Plan(format!(
        "Invalid aggregation '{}'",
        name,
    )));
}

@alamb
Copy link
Contributor Author

alamb commented Jun 3, 2021

Thanks @jgoday ! Perhaps you can make the error a little more specific -- like "no arguments passed to aggregate {}" or something. But the location looks reasonable

@jgoday
Copy link
Contributor

jgoday commented Jun 4, 2021

Thank you @alamb. I have created a PR #505 for this.
How about this error message?

Aggregate error. Invalid or wrong number of arguments passed to aggregate: {}

@alamb
Copy link
Contributor Author

alamb commented Jun 4, 2021

I think "Invalid or wrong number of arguments passed to aggregate: {}" is probably sufficient

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

Successfully merging a pull request may close this issue.

2 participants