-
Notifications
You must be signed in to change notification settings - Fork 764
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 FixedSizedListArray
for length
kernel
#4520
feat: Support FixedSizedListArray
for length
kernel
#4520
Conversation
ab3362d
to
b80afa0
Compare
arrow-string/src/length.rs
Outdated
let length_list = array.len(); | ||
let buffer = Buffer::from_vec(vec![length; length_list]); | ||
|
||
let data = unsafe { |
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.
Perhaps we could use PrimitiveArray::new here?
Hello, I just reviewed this PR and I have two suggestions:
Otherwise it looks good to me! |
arrow-string/src/length.rs
Outdated
let length_list = array.len(); | ||
let buffer = Buffer::from_vec(vec![length; length_list]); | ||
|
||
let data: PrimitiveArray<T> = |
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 remove T and just use Int32Array?
{ | ||
let array = array.as_any().downcast_ref::<FixedSizeListArray>().unwrap(); | ||
let length_list = array.len(); | ||
let buffer = Buffer::from_vec(vec![length; length_list]); |
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.
This assumes T::Native is i32
arrow-string/src/length.rs
Outdated
let data: PrimitiveArray<T> = | ||
PrimitiveArray::new(buffer.into(), array.nulls().cloned()); | ||
|
||
make_array(data.into()) |
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.
make_array(data.into()) | |
Arc::new(data) |
arrow-string/src/length.rs
Outdated
where | ||
T: ArrowPrimitiveType, | ||
{ | ||
let array = array.as_any().downcast_ref::<FixedSizeListArray>().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.
let array = array.as_any().downcast_ref::<FixedSizeListArray>().unwrap(); | |
let array = array.as_fixed_size_list(); |
Which issue does this PR close?
Closes #4517
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?