Avoid error with empty iterators used for ScalarValue::iter_to_array
#4206
Conversation
The size of the iterator that was passed to the the `iter_to_array` wasn't always checked. Which could cause an error, because the function couldn't figure out the `DataType` of the array its supposed to generate.
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
This change makes sense to me -- thank you @GrandChaman
I reviewed https://github.com/apache/arrow-datafusion/blob/28ca3eeac437994be836d299748d3fb3e0ed8b04/datafusion/core/src/physical_plan/aggregates/hash.rs#L425-L450 and I don't think it is affected so we don't need to make changes there
alamb
changed the title
ScalarValue::iter_to_arrayScalarValue::iter_to_array
|
Thanks again @GrandChaman |
|
Benchmark runs are scheduled for baseline = 13127ae and contender = 88c1201. 88c1201 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
Closes #4080.
Rationale for this change
I've noticed that some calls to
ScalarValue::iter_to_arraycould have an empty iterator as argument, which caused an error.What changes are included in this PR?
In 2 places I found out that sometimes,
ScalarValue::iter_to_arrayis called with an empty iterator. I made sure to usearrow::array::new_empty_arrayin this case.Are these changes tested?
One of them is (the one about the
ScalarValue::List).I didn't find a good way to test the other one, as it seems to require some more insights into how the aggregation works internally.
Are there any user-facing changes?
No, and no breaking changes.