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/08/03 11:55:25 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #3026: Split binary_expr.rs into smaller modules
alamb opened a new pull request, #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026
NOTE: This change looks large but it is only moving code around
# Which issue does this PR close?
re https://github.com/apache/arrow-datafusion/issues/1474
# Rationale for this change
I find it really hard to work on `binary.rs`. Partly this is because what it is doing is complicated however, I think it can be less complicated than currently, and thus I am trying to unravel the [Gordian knot](https://www.merriam-webster.com/dictionary/Gordian%20knot)
Part of why this module is complicated is because it has both dispatch logic (figure out what functions to call based on argument types) as well as kernel implementations (both stuff that should eventually end up in arrow as well as things that are datafusion specific).
I think it will be easier to work with if the file were smaller.
# What changes are included in this PR?
Thus, break binary.rs into several smaller modules:
* `binary.rs`: The main dispatch logic
* `binary/kernels.rs`: kernels that are unique to datafusion
* `binary/arrow_kernels.rs`: Kernels that are eventually destined for arrow-rs but are in datafusion until we get around tot hat
* `binary/adapte.rs`: functions that might change types or names to make them compatible with the main dispatch logic
# Are there any user-facing changes?
No
--
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] codecov-commenter commented on pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#issuecomment-1203885732
# [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/3026?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3026](https://codecov.io/gh/apache/arrow-datafusion/pull/3026?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62e2e70) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/9a5f17e6bbc9b8acbb0d0e584ebc9ade291622f2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9a5f17e) will **increase** coverage by `0.00%`.
> The diff coverage is `90.43%`.
```diff
@@ Coverage Diff @@
## master #3026 +/- ##
=======================================
Coverage 85.84% 85.85%
=======================================
Files 283 286 +3
Lines 51658 51666 +8
=======================================
+ Hits 44347 44356 +9
+ Misses 7311 7310 -1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/3026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [datafusion/physical-expr/src/expressions/binary.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9iaW5hcnkucnM=) | `97.88% <ø> (+2.17%)` | :arrow_up: |
| [...on/physical-expr/src/expressions/binary/kernels.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9iaW5hcnkva2VybmVscy5ycw==) | `72.00% <72.00%> (ø)` | |
| [...on/physical-expr/src/expressions/binary/adapter.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9iaW5hcnkvYWRhcHRlci5ycw==) | `83.33% <83.33%> (ø)` | |
| [...sical-expr/src/expressions/binary/kernels\_arrow.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9iaW5hcnkva2VybmVsc19hcnJvdy5ycw==) | `94.02% <94.02%> (ø)` | |
| [datafusion/common/src/scalar.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb21tb24vc3JjL3NjYWxhci5ycw==) | `84.81% <0.00%> (-0.07%)` | :arrow_down: |
| [datafusion/expr/src/logical\_plan/plan.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9sb2dpY2FsX3BsYW4vcGxhbi5ycw==) | `77.77% <0.00%> (ø)` | |
| [datafusion/core/src/physical\_plan/metrics/value.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL21ldHJpY3MvdmFsdWUucnM=) | `87.43% <0.00%> (+0.50%)` | :arrow_up: |
| [datafusion/expr/src/window\_frame.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy93aW5kb3dfZnJhbWUucnM=) | `93.27% <0.00%> (+0.84%)` | :arrow_up: |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] yjshen commented on pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
yjshen commented on PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#issuecomment-1204714481
An alternative for splitting `binary.rs` I could think of:
- `arithmetic.rs` for +/-/*/div/mod calculations.
- `predicate.rs` for expressions that evals to boolean.
- `bitwise.rs` for bit calculations.
- `regex.rs` for regex
- and `misc.rs` for the remainings.
--
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 diff in pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#discussion_r937996173
##########
datafusion/physical-expr/src/expressions/binary/kernels.rs:
##########
@@ -0,0 +1,191 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! This module contains computation kernels that are specific to
+//! datafusion and not (yet) targeted to port upstream to arrow
+use arrow::array::*;
+use arrow::datatypes::DataType;
+use datafusion_common::{DataFusionError, Result, ScalarValue};
+use datafusion_expr::Operator;
+use std::sync::Arc;
+
+/// The binary_bitwise_array_op macro only evaluates for integer types
+/// like int64, int32.
+/// It is used to do bitwise operation.
+macro_rules! binary_bitwise_array_op {
+ ($LEFT:expr, $RIGHT:expr, $OP:tt, $ARRAY_TYPE:ident, $TYPE:ty) => {{
+ let len = $LEFT.len();
+ let left = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let right = $RIGHT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let result = (0..len)
+ .into_iter()
+ .map(|i| {
+ if left.is_null(i) || right.is_null(i) {
+ None
+ } else {
+ Some(left.value(i) $OP right.value(i))
+ }
+ })
+ .collect::<$ARRAY_TYPE>();
+ Ok(Arc::new(result))
+ }};
+}
+
+/// The binary_bitwise_array_op macro only evaluates for integer types
+/// like int64, int32.
+/// It is used to do bitwise operation on an array with a scalar.
+macro_rules! binary_bitwise_array_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:tt, $ARRAY_TYPE:ident, $TYPE:ty) => {{
+ let len = $LEFT.len();
+ let array = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let scalar = $RIGHT;
+ if scalar.is_null() {
+ Ok(new_null_array(array.data_type(), len))
+ } else {
+ let right: $TYPE = scalar.try_into().unwrap();
+ let result = (0..len)
+ .into_iter()
+ .map(|i| {
+ if array.is_null(i) {
+ None
+ } else {
+ Some(array.value(i) $OP right)
+ }
+ })
+ .collect::<$ARRAY_TYPE>();
+ Ok(Arc::new(result) as ArrayRef)
+ }
+ }};
+}
+
+pub(crate) fn bitwise_and(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
+ match &left.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_op!(left, right, &, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_op!(left, right, &, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_op!(left, right, &, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_op!(left, right, &, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseAnd
+ ))),
+ }
+}
+
+pub(crate) fn bitwise_or(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
+ match &left.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_op!(left, right, |, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_op!(left, right, |, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_op!(left, right, |, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_op!(left, right, |, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseOr
+ ))),
+ }
+}
+
+pub(crate) fn bitwise_and_scalar(
+ array: &dyn Array,
+ scalar: ScalarValue,
+) -> Option<Result<ArrayRef>> {
+ let result = match array.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseAnd
+ ))),
+ };
+ Some(result)
+}
+
+pub(crate) fn bitwise_or_scalar(
+ array: &dyn Array,
+ scalar: ScalarValue,
+) -> Option<Result<ArrayRef>> {
+ let result = match array.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseOr
+ ))),
+ };
+ Some(result)
+}
+
+/// Concat lhs and rhs String Array, any `NULL` exists on lhs or rhs will come out result `NULL`
+/// 1. 'a' || 'b' || 32 = 'ab32'
+/// 2. 'a' || NULL = NULL
+pub(crate) fn string_concat(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
Review Comment:
🤔 it seems like this may now be part of arrow:
https://docs.rs/arrow/19.0.0/arrow/compute/kernels/concat_elements/fn.concat_elements_utf8.html
I'll and switch to use that as a follow on PR
--
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 diff in pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#discussion_r938036155
##########
datafusion/physical-expr/src/expressions/binary/kernels.rs:
##########
@@ -0,0 +1,191 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! This module contains computation kernels that are specific to
+//! datafusion and not (yet) targeted to port upstream to arrow
+use arrow::array::*;
+use arrow::datatypes::DataType;
+use datafusion_common::{DataFusionError, Result, ScalarValue};
+use datafusion_expr::Operator;
+use std::sync::Arc;
+
+/// The binary_bitwise_array_op macro only evaluates for integer types
+/// like int64, int32.
+/// It is used to do bitwise operation.
+macro_rules! binary_bitwise_array_op {
+ ($LEFT:expr, $RIGHT:expr, $OP:tt, $ARRAY_TYPE:ident, $TYPE:ty) => {{
+ let len = $LEFT.len();
+ let left = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let right = $RIGHT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let result = (0..len)
+ .into_iter()
+ .map(|i| {
+ if left.is_null(i) || right.is_null(i) {
+ None
+ } else {
+ Some(left.value(i) $OP right.value(i))
+ }
+ })
+ .collect::<$ARRAY_TYPE>();
+ Ok(Arc::new(result))
+ }};
+}
+
+/// The binary_bitwise_array_op macro only evaluates for integer types
+/// like int64, int32.
+/// It is used to do bitwise operation on an array with a scalar.
+macro_rules! binary_bitwise_array_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:tt, $ARRAY_TYPE:ident, $TYPE:ty) => {{
+ let len = $LEFT.len();
+ let array = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
+ let scalar = $RIGHT;
+ if scalar.is_null() {
+ Ok(new_null_array(array.data_type(), len))
+ } else {
+ let right: $TYPE = scalar.try_into().unwrap();
+ let result = (0..len)
+ .into_iter()
+ .map(|i| {
+ if array.is_null(i) {
+ None
+ } else {
+ Some(array.value(i) $OP right)
+ }
+ })
+ .collect::<$ARRAY_TYPE>();
+ Ok(Arc::new(result) as ArrayRef)
+ }
+ }};
+}
+
+pub(crate) fn bitwise_and(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
+ match &left.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_op!(left, right, &, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_op!(left, right, &, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_op!(left, right, &, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_op!(left, right, &, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseAnd
+ ))),
+ }
+}
+
+pub(crate) fn bitwise_or(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
+ match &left.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_op!(left, right, |, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_op!(left, right, |, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_op!(left, right, |, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_op!(left, right, |, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseOr
+ ))),
+ }
+}
+
+pub(crate) fn bitwise_and_scalar(
+ array: &dyn Array,
+ scalar: ScalarValue,
+) -> Option<Result<ArrayRef>> {
+ let result = match array.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_scalar!(array, scalar, &, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseAnd
+ ))),
+ };
+ Some(result)
+}
+
+pub(crate) fn bitwise_or_scalar(
+ array: &dyn Array,
+ scalar: ScalarValue,
+) -> Option<Result<ArrayRef>> {
+ let result = match array.data_type() {
+ DataType::Int8 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int8Array, i8)
+ }
+ DataType::Int16 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int16Array, i16)
+ }
+ DataType::Int32 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int32Array, i32)
+ }
+ DataType::Int64 => {
+ binary_bitwise_array_scalar!(array, scalar, |, Int64Array, i64)
+ }
+ other => Err(DataFusionError::Internal(format!(
+ "Data type {:?} not supported for binary operation '{}' on dyn arrays",
+ other,
+ Operator::BitwiseOr
+ ))),
+ };
+ Some(result)
+}
+
+/// Concat lhs and rhs String Array, any `NULL` exists on lhs or rhs will come out result `NULL`
+/// 1. 'a' || 'b' || 32 = 'ab32'
+/// 2. 'a' || NULL = NULL
+pub(crate) fn string_concat(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
Review Comment:
https://github.com/apache/arrow-datafusion/pull/3036
--
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 merged pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
alamb merged PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026
--
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 #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#issuecomment-1205499585
> An alternative for splitting binary.rs I could think of:
Thanks @yjshen -- my ideal world is that there are no actual calculation kernels in datafusion (that they are all in arrow) but if we get more kernels into data fusion the split you suggest would be good.
Or maybe I can split `kernels.rs` into more submodules 🤔
--
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 #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#issuecomment-1204429064
cc @avantgardnerio in case you have made changes to the decimal code as part of the `arrow 20` possible upgrade
--
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] ursabot commented on pull request #3026: Split `binary_expr.rs` into smaller modules
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #3026:
URL: https://github.com/apache/arrow-datafusion/pull/3026#issuecomment-1205508934
Benchmark runs are scheduled for baseline = 3d27987703b7602b606a77caa08696c7b3da5daf and contender = d07a775c55091195a3f615db61e49b1ef97ce5ba. d07a775c55091195a3f615db61e49b1ef97ce5ba is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a562fc416cf340b0a93ca2753092b334...41edae5585f54a46a9890de0bb17cb8d/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/a9abddb5481b4d00b84727aae03426db...f1086e733f994527bcf39dc7c20ed5d7/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a45d57b473d4403c941ad5b67fd5dbb5...96f5ff55ef454b49a7e2622408d17ac6/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/916082733c78474fb1472fe3f539770d...6389f5f7f92449c29703f9f3bf70cd39/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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