-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: array_resize null fix #13209
fix: array_resize null fix #13209
Conversation
// Checks if entire array is null | ||
if array.null_count() == array.len() { |
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.
array
is an array of arrays, right?
what happens if some values are null and some other are not?
For example a query processes some null and non-null arrays.
currently, array_resize converts second array (being null) into an empty array:
> SELECT a, array_resize(a, 1) FROM (VALUES (make_array(1)), (arrow_cast(NULL, 'List(Int32)')), (make_array(3))) t(a);
+-----+----------------------------+
| a | array_resize(t.a,Int64(1)) |
+-----+----------------------------+
| [1] | [1] |
| | [] | <!-------- should be NULL
| [3] | [3] |
+-----+----------------------------+
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.
Thanks for pointing that out, I overlooked that.
@@ -7013,6 +7013,12 @@ select array_resize(arrow_cast([[1], [2], [3]], 'LargeList(List(Int64))'), 10, [ | |||
---- | |||
[[1], [2], [3], [5], [5], [5], [5], [5], [5], [5]] | |||
|
|||
# array_resize null value | |||
query ? | |||
select array_resize(arrow_cast(NULL, 'List(Int8)'), 1); |
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.
Can we also add columnar test?
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 kept two null checks in the array_resize function: one in the array_resize_inner function and one in the general_list_resize function. The first null check is needed for the original problem of handling cases like SELECT array_resize(arrow_cast(NULL, 'List(Int8)'), 1);. This scenario can't be checked by the second null check because the array has a length of zero. With a length of zero, it conflicts with the null builder, as the null counts as a length of one, which isn't the expected value when constructing the result. This is due to the null array and the data array needing to have matching lengths. |
This comment was marked as outdated.
This comment was marked as outdated.
lgtm @alamb |
Thanks @jonathanc-n @findepi @jatin510 |
Which issue does this PR close?
Closes #13200 .
Rationale for this change
What changes are included in this PR?
Added null check for array_resize
Are these changes tested?
Are there any user-facing changes?