add lt_bool, lt_eq_bool, gt_bool, gt_eq_bool #860
Conversation
jimexist
force-pushed
the
add-boolean-lt
branch
from
74503fb to
2903c7d
Compare
|
Looks good! I think we should add the scalar versions too (lt_bool_scalar, etc)? |
i don't think we need.
|
Codecov Report
@@ Coverage Diff @@
## master #860 +/- ##
==========================================
- Coverage 82.67% 82.66% -0.01%
==========================================
Files 168 168
Lines 48129 48172 +43
==========================================
+ Hits 39792 39823 +31
- Misses 8337 8349 +12
Continue to review full report at Codecov.
|
| /// Perform `left == right` operation on [`BooleanArray`] | ||
| fn eq_bool(left: &BooleanArray, right: &BooleanArray) -> Result<BooleanArray> { | ||
| #[inline] | ||
| fn binary_boolean_op<F>( |
There was a problem hiding this comment.
Is it worth a doc comment that says that op is performed 64 bits at a time withu64 (aka so it is not possible to use a < b for example, to implement lt_bool)
|
|
||
| /// Perform `left < right` operation on [`BooleanArray`] | ||
| fn lt_bool(left: &BooleanArray, right: &BooleanArray) -> Result<BooleanArray> { | ||
| binary_boolean_op(left, right, |a, b| ((!a) & b)) |
There was a problem hiding this comment.
👍
the truth table shows it to be true:
| a | b | a < b |
|---|---|---|
| t | f | f |
| t | f | f |
| f | t | t |
| f | t | f |
I checked the others as well
|
|
||
| /// Perform `left <= right` operation on [`BooleanArray`] | ||
| fn lt_eq_bool(left: &BooleanArray, right: &BooleanArray) -> Result<BooleanArray> { | ||
| binary_boolean_op(left, right, |a, b| !(a & (!b))) |
There was a problem hiding this comment.
This can also be done with two operations rather than three via (!a | b) but I suspect the compiler will do that transformation if it is useful
|
|
||
| /// Perform `left >= right` operation on [`BooleanArray`] | ||
| fn gt_eq_bool(left: &BooleanArray, right: &BooleanArray) -> Result<BooleanArray> { | ||
| binary_boolean_op(left, right, |a, b| !((!a) & b)) |
There was a problem hiding this comment.
likewise could also be written like (a | !b)
Co-authored-by: Jiayu Liu <[email protected]>
4 participants
Which issue does this PR close?
Following #844
Closes #861
Rationale for this change
add lt_bool, lt_eq_bool, gt_bool, gt_eq_bool
What changes are included in this PR?
Are there any user-facing changes?