feat: support bitwise shift left/right
#4148
Conversation
Weijun-H
changed the title
bitwise shift left/right
| @@ -143,6 +177,24 @@ mod tests { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
Is it worth adding a test of what happens if you shift by more than the number of bits?
There was a problem hiding this comment.
I add a test for it. It should be 0
There was a problem hiding this comment.
There was a problem hiding this comment.
This makes sense, as wrapping_shr would shift the position would be the remainder of the type. So wrapping_shr(3_i32, 32) == 3 and wrapping_shr(3_i32, 33) == 1
There was a problem hiding this comment.
Shift operations (<<, >>) on a value of width N can be passed a shift value >= N. It is unclear what behaviour should result from this, so the shift value is unconditionally masked to be modulo N to ensure that the argument is always in range.
I found the reason here https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md
| use arrow_schema::ArrowError; | ||
| use num::traits::{WrappingShl, WrappingShr}; |
There was a problem hiding this comment.
this is great -- when I looked into doing this in DataFusion I noticed these traits weren't in std -- 100% for finding them in num
Which issue does this PR close?
Closes #2741
Rationale for this change
Support
bitwise_shift_rightandbitwise_shift_left. And use 'as_usize' to ignore truncation.What changes are included in this PR?
Are there any user-facing changes?