Use ArrayAccessor in Comparison Kernels #2157
Conversation
| Ok(BooleanArray::from(data)) | ||
| }}; | ||
| } | ||
| fn compare_op<T: ArrayAccessor, F>(left: T, right: T, op: F) -> Result<BooleanArray> |
There was a problem hiding this comment.
ArrayAccessor allows us to replace macro with generic function like this.
| (DataType::Int8, DataType::Int8) => compare_op( | ||
| left.as_any().downcast_ref::<Int8Array>().unwrap(), | ||
| right.as_any().downcast_ref::<Int8Array>().unwrap(), | ||
| |a, b| a == b, | ||
| ), |
There was a problem hiding this comment.
But for some macros which we match array types, seems it doesn't reduce too much duplication, as we still need to match particular array type (which implements ArrayAccessor).
Codecov Report
@@ Coverage Diff @@
## master #2157 +/- ##
========================================
Coverage 82.86% 82.86%
========================================
Files 237 237
Lines 61296 61426 +130
========================================
+ Hits 50792 50901 +109
- Misses 10504 10525 +21
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
|
|
Nice improvement! |
|
Benchmark runs are scheduled for baseline = e68852c and contender = 4a47ab7. 4a47ab7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
Thanks @jhorstmann @alamb @tustvold |
Which issue does this PR close?
Closes #2135.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?