-
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
feat: support LargeList
in flatten
#9110
Conversation
FixedSizeList
in flatten
LargeList
in flatten
@@ -551,7 +551,12 @@ impl BuiltinScalarFunction { | |||
DataType::List(_) => get_base_type(field.data_type()), | |||
_ => Ok(data_type.to_owned()), | |||
}, | |||
_ => internal_err!("Not reachable, data_type should be List"), | |||
DataType::LargeList(field) => match field.data_type() { |
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.
we prob can use if guard here?
DataType::LargeList(field) if field.data_type() =>
offsets: OffsetBuffer<i32>, | ||
indexes: OffsetBuffer<i32>, | ||
) -> OffsetBuffer<i32> { | ||
fn get_offsets_for_flatten<O: OffsetSizeTrait>( |
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.
👍
let offsets: Vec<i32> = indexes.iter().map(|i| buffer[*i as usize]).collect(); | ||
let offsets: Vec<O> = indexes | ||
.iter() | ||
.map(|i| buffer[i.to_usize().unwrap()]) |
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.
wondering what is cheaper....
just cast as usize, or to_size and then unwrap
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.
We cannot cast it as usize directly because *i
is OffsetSizeTrait
query ??? | ||
select flatten(make_array(1, 2, 1, 3, 2)), | ||
flatten(make_array([1], [2, 3], [null], make_array(4, null, 5))), | ||
flatten(make_array([[1.1]], [[2.2]], [[3.3], [4.4]])); | ||
---- | ||
[1, 2, 1, 3, 2] [1, 2, 3, , 4, , 5] [1.1, 2.2, 3.3, 4.4] | ||
|
||
query ??? |
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.
please add test description
query ???? | ||
select column1, column2, column3, column4 from flatten_table; | ||
---- | ||
[[1], [2], [3]] [[[1, 2, 3]], [[4, 5]], [[6]]] [[[[1]]], [[[2, 3]]]] [[1.0], [2.1, 2.2], [3.2, 3.3, 3.4]] | ||
[[1, 2], [3, 4], [5, 6]] [[[8]]] [[[[1, 2]]], [[[3]]]] [[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]] | ||
|
||
query ???? |
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.
please add test description
@@ -5368,6 +5398,16 @@ from flatten_table; | |||
[1, 2, 3] [1, 2, 3, 4, 5, 6] [1, 2, 3] [1.0, 2.1, 2.2, 3.2, 3.3, 3.4] | |||
[1, 2, 3, 4, 5, 6] [8] [1, 2, 3] [1.0, 2.0, 3.0, 4.0, 5.0, 6.0] | |||
|
|||
query ???? |
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.
ditto
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 @Weijun-H good PR, please take care on minors
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.
lgtm thanks @Weijun-H
Which issue does this PR close?
Parts #8185
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?