alamb commented on a change in pull request #1249:
URL: https://github.com/apache/arrow-rs/pull/1249#discussion_r795177165
##########
File path: arrow/src/compute/kernels/take.rs
##########
@@ -496,27 +496,30 @@ where
IndexType: ArrowNumericType,
IndexType::Native: ToPrimitive,
{
- // TODO optimize decimal take and construct decimal array from
MutableBuffer
- let mut builder = DecimalBuilder::new(
- indices.len(),
- decimal_values.precision(),
- decimal_values.scale(),
- );
- for i in 0..indices.len() {
- if indices.is_null(i) {
- builder.append_null()?;
- } else {
- let index = ToPrimitive::to_usize(&indices.value(i)).ok_or_else(||
{
- ArrowError::ComputeError("Cast to usize failed".to_string())
- })?;
- if decimal_values.is_null(index) {
- builder.append_null()?
- } else {
- builder.append_value(decimal_values.value(index))?
- }
- }
- }
- Ok(builder.finish())
+ indices
+ .iter()
+ .map(|index| {
+ // Use type annotations below for readability (was blowing
+ // my mind otherwise)
+ let t: Option<Result<Option<_>>> = index.map(|index| {
+ let index = ToPrimitive::to_usize(&index).ok_or_else(|| {
+ ArrowError::ComputeError("Cast to usize
failed".to_string())
+ })?;
+
+ if decimal_values.is_null(index) {
+ Ok(None)
+ } else {
+ Ok(Some(decimal_values.value(index)))
+ }
+ });
+ let t: Result<Option<Option<_>>> = t.transpose();
+ let t: Result<Option<_>> = t.map(|t| t.flatten());
+ t
+ })
+ .collect::<Result<DecimalArray>>()?
+ // PERF: we could avoid re-validating that the data in
Review comment:
this re-validation occurs in DecimalBuilder as well, so this change
isn't worse. I just noticed it while working on the code and figured I would
leave a hint for future readers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
