You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/28 16:48:38 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request #2107: Minor: Reduce repetition in Decimal binary kernels
alamb opened a new pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107
# Rationale
I noticed some things that could be written "more nicely" while trolling through the code for other reasons and figured I would sneak in some coding (rather than review)
I think this also makes it easier to port this code to arrow-rs eventually: https://github.com/apache/arrow-rs/issues/1200
Changes:
1. Remove repetition in the code
--
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
[GitHub] [arrow-datafusion] alamb commented on pull request #2107: Minor: Reduce repetition in Decimal binary kernels
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107#issuecomment-1082147332
Blocked by upstream issue in arrow-rs: https://github.com/apache/arrow-rs/pull/1506
--
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
[GitHub] [arrow-datafusion] alamb commented on pull request #2107: Minor: Reduce repetition in Decimal binary kernels
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107#issuecomment-1080901020
FYI @liukun4515
--
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
[GitHub] [arrow-datafusion] alamb commented on pull request #2107: Minor: Reduce repetition in Decimal binary kernels
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107#issuecomment-1080964160
Draft until I can debug CI failure
--
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
[GitHub] [arrow-datafusion] liukun4515 commented on a change in pull request #2107: Minor: Reduce repetition in Decimal binary kernels
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on a change in pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107#discussion_r838047597
##########
File path: datafusion-physical-expr/src/expressions/binary.rs
##########
@@ -135,260 +135,192 @@ fn is_not_distinct_from_bool(
// TODO move decimal kernels to to arrow-rs
// https://github.com/apache/arrow-rs/issues/1200
-// TODO use iter added for for decimal array in
-// https://github.com/apache/arrow-rs/issues/1083
+/// Creates an BooleanArray the same size as `left`,
+/// applying `op` to all non-null elements of left
+fn compare_decimal_scalar<F>(
+ left: &DecimalArray,
+ right: i128,
+ op: F,
+) -> Result<BooleanArray>
+where
+ F: Fn(i128, i128) -> bool,
+{
+ Ok(left
+ .iter()
+ .map(|left| left.map(|left| op(left, right)))
Review comment:
Does this iter need `sized`？
--
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
[GitHub] [arrow-datafusion] alamb commented on a change in pull request #2107: Minor: Reduce repetition in Decimal binary kernels
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #2107:
URL: https://github.com/apache/arrow-datafusion/pull/2107#discussion_r836641657
##########
File path: datafusion-physical-expr/src/expressions/binary.rs
##########
@@ -135,260 +135,192 @@ fn is_not_distinct_from_bool(
// TODO move decimal kernels to to arrow-rs
// https://github.com/apache/arrow-rs/issues/1200
-// TODO use iter added for for decimal array in
-// https://github.com/apache/arrow-rs/issues/1083
+/// Creates an BooleanArray the same size as `left`,
+/// applying `op` to all non-null elements of left
+fn compare_decimal_scalar<F>(
+ left: &DecimalArray,
+ right: i128,
+ op: F,
+) -> Result<BooleanArray>
+where
+ F: Fn(i128, i128) -> bool,
+{
+ Ok(left
+ .iter()
+ .map(|left| left.map(|left| op(left, right)))
+ .collect())
+}
+
+/// Creates an BooleanArray the same size as `left`,
+/// by applying `op` to all non-null elements of left and right
+fn compare_decimal<F>(
+ left: &DecimalArray,
+ right: &DecimalArray,
+ op: F,
+) -> Result<BooleanArray>
+where
+ F: Fn(i128, i128) -> bool,
+{
+ Ok(left
+ .iter()
+ .zip(right.iter())
+ .map(|(left, right)| {
+ if let (Some(left), Some(right)) = (left, right) {
+ Some(op(left, right))
+ } else {
+ None
+ }
+ })
+ .collect())
+}
+
pub(super) fn eq_decimal_scalar(
left: &DecimalArray,
right: i128,
) -> Result<BooleanArray> {
- let mut bool_builder = BooleanBuilder::new(left.len());
- for i in 0..left.len() {
- if left.is_null(i) {
- bool_builder.append_null()?;
- } else {
- bool_builder.append_value(left.value(i) == right)?;
- }
- }
- Ok(bool_builder.finish())
+ compare_decimal_scalar(left, right, |left, right| left == right)
Review comment:
This PR simply refactors out the common iteration over inputs into `compare_decimal` and `compare_decimal_scalar`
--
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