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
support CastOption
for casting numeric
#2649
support CastOption
for casting numeric
#2649
Conversation
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.
Makes sense to me -- thank you @liukun4515
}) | ||
.collect::<Result<Vec<Option<R::Native>>>>()?; | ||
|
||
Ok(unsafe { PrimitiveArray::<R>::from_trusted_len_iter(iter) }) |
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.
You could use try_from_trusted_len_iter to avoid needing to create the intermediate vec
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.
Do you mean use the try_from_trusted_len_iter
API for the MutableBuffer
or Buffer
?
Does it can fit this result?
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.
Do you mean add a new api try_xxx
for PrimitiveArray
?
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 can use bellow code to implement
let iter = from
.iter()
.map(|v| match v {
None => Ok(None),
Some(value) => match num::cast::cast::<T::Native, R::Native>(value) {
None => Err(ArrowError::CastError(format!(
"Can't cast value {:?} to type {}",
value,
R::DATA_TYPE
))),
Some(v) => Ok(Some(v)),
},
});
Buffer::try_from_trusted_len_iter(iter)
let data = unsafe {
ArrayData::new_unchecked(
T::DATA_TYPE,
len,
None,
null_bit_buffer,
0,
vec![buffer],
vec![],
)
};
Ok(PrimitiveArray::<T>::from(data))
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.
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 don't think the code above is correct, in particular you are taking an iterator of Option and collecting into a Buffer. I would not expect it to even compile...
I had thought PrimitiveArray had a try_from_trysted_len_iter but it does not, so ignore me. The real performance will likely be from not using iterators at all, but one step at a time and I don't imagine the cast kernels are a major bottleneck atm
Benchmark runs are scheduled for baseline = 773f1b9 and contender = 8ea6ca1. 8ea6ca1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2648
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?