[GitHub] [arrow-rs] alamb commented on a change in pull request #1249: Use new DecimalArray creation API in arrow crate

[GitBox <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org