Skip to content

fix: array_slice panics #10547

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 17, 2024
Merged

fix: array_slice panics #10547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65a696c
fix: array_slice panics
jonahgao May 16, 2024
112d1b4
add test
jonahgao May 16, 2024
8a94a03
add test
jonahgao May 16, 2024
4f46694
update test
jonahgao May 16, 2024
3ec6b1e
fix comment
jonahgao May 16, 2024
3f247ab
fix clippy
jonahgao May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions datafusion/functions-array/src/extract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,19 +418,16 @@ where

if let (Some(from), Some(to)) = (from_index, to_index) {
let stride = stride.map(|s| s.value(row_index));
// array_slice with stride in duckdb, return empty array if stride is not supported and from > to.
if stride.is_none() && from > to {
// return empty array
offsets.push(offsets[row_index]);
continue;
}
// Default stride is 1 if not provided
let stride = stride.unwrap_or(1);
if stride.is_zero() {
return exec_err!(
"array_slice got invalid stride: {:?}, it cannot be 0",
stride
);
} else if from <= to && stride.is_negative() {
} else if (from <= to && stride.is_negative())
Copy link
Member Author

@jonahgao jonahgao May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When from is equal to to, it should not return an empty list.
The fix for it is not included in this PR, it has been added to issue #10548.

|| (from > to && stride.is_positive())
{
// return empty array
offsets.push(offsets[row_index]);
continue;
Expand Down
12 changes: 12 additions & 0 deletions datafusion/sqllogictest/test_files/array.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1963,6 +1963,18 @@ select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5), 'LargeList(Int64)'), co
[1, 2, 3, 4, 5] [43, 44, 45, 46] [41, 42, 43, 44, 45]
[5] [, 54, 55, 56, 57, 58, 59, 60] [55]

# Test issue: https://github.com/apache/datafusion/issues/10425
# `from` may be larger than `to` and `stride` is positive
query ????
select array_slice(a, -1, 2, 1), array_slice(a, -1, 2),
array_slice(a, 3, 2, 1), array_slice(a, 3, 2)
from (values ([1.0, 2.0, 3.0, 3.0]), ([4.0, 5.0, 3.0]), ([6.0])) t(a);
----
[] [] [] []
[] [] [] []
[6.0] [6.0] [] []


# make_array with nulls
query ???????
select make_array(make_array('a','b'), null),
Expand Down