-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix filter_record_batch panics with empty struct array
#7539
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
Conversation
9c9c598 to
7587b6d
Compare
exists from the predicate. This prevents panics in situations where a stucts array has empty columns.
7587b6d to
bbad99f
Compare
|
cc @alamb |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| #[test] | ||
| fn test_filter_empty_struct() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified that this test fails without the code in this PR:
thread 'filter::tests::test_filter_empty_struct' panicked at arrow-array/src/array/struct_array.rs:224:51:
cannot use StructArray::new_unchecked if there are no fields, length is unknown
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
I think this is additional fallout from a new panic we introduced in this PR from @westonpace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct however without that panic the bug was much more subtle as it created an invalid record batch that would panic when slicing elsewhere.
StructArraysfilter_record_batch panics with empty struct array. #7538
filter_record_batch panics with empty struct array. #7538filter_record_batch panics with empty struct array
|
Thanks again @thorfour |
* unit test: filter empty struct * When filtering struct Array's always build with a length since one exists from the predicate. This prevents panics in situations where a stucts array has empty columns.
Which issue does this PR close?
Rationale for this change
This prevents a panic that can happen with empty struct arrays.
What changes are included in this PR?
Small unit test that exacerbates the panic by attempting to filter a struct array with no columns.
Fix was to call
StructArray:new_unchecked_with_lengthsince the predicate of the filter contains the number rows the array requires.Are there any user-facing changes?
No