implement eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn#858
implement eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn#858jimexist merged 1 commit intoapache:masterfrom
eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn#858Conversation
jimexist
changed the title
eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn
jimexist
force-pushed
the
impl-eq-neq-dyn
branch
from
84250ae to
7179a44
Compare
Codecov Report
@@ Coverage Diff @@
## master #858 +/- ##
==========================================
- Coverage 82.66% 82.65% -0.02%
==========================================
Files 168 168
Lines 48172 48196 +24
==========================================
+ Hits 39823 39834 +11
- Misses 8349 8362 +13
Continue to review full report at Codecov.
|
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
I think this looks really nice @jimexist -- thank you!
FWIW I took a friendly look at what arrow2 does for this situation and I think this PR is not inconsistent. Arrow2 appears to have a function called arithmetic which does the type dispatch on the parameter types and an Operator enum:
https://github.com/jorgecarleitao/arrow2/blob/main/src/compute/arithmetics/mod.rs#L91
While an Operator enum seems like a reasonable thing to consider adding to arrow-rs it is also likely fine to do as a follow on (as we already have different kernels for different operations)
jimexist
force-pushed
the
impl-eq-neq-dyn
branch
from
e6c66aa to
08b53e5
Compare
Co-authored-by: Jiayu Liu <[email protected]>
4 participants
Which issue does this PR close?
Closes #843
Rationale for this change
to reduce casting on the call sites
What changes are included in this PR?
implement:
eq_dynneq_dynlt_dynlt_eq_dyngt_dyngt_eq_dynAre there any user-facing changes?