Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(expr): Implementation for bitwise operation #2884

Merged
merged 57 commits into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
43baed2
Add bitwise_op.rs and implement << and >>
marvenlee2486 May 19, 2022
8d81e88
Add ExprType and Do binding of BinaryOperator to ExprType
marvenlee2486 May 19, 2022
1137e7f
ModifeModified expr and frontend
marvenlee2486 May 20, 2022
b330586
Working for >> and <<
marvenlee2486 May 23, 2022
b1b3095
Trying to add bitand but facing problem
marvenlee2486 May 23, 2022
4a82382
Done implementation for bitwise and or XOR(#), TODO ~ and Unit Test a…
marvenlee2486 May 23, 2022
b8893b9
Edit Bitwise not But still have server closed unexpected error
marvenlee2486 May 25, 2022
748ae00
Bitwise not done implemetation. TODO Testing
marvenlee2486 May 25, 2022
00662e1
Done Unit testing, and done code e2e test
marvenlee2486 May 28, 2022
3fcbe92
Done Unit testing, and done code e2e test
marvenlee2486 May 28, 2022
93c3b23
last commit
marvenlee2486 May 29, 2022
a2011f8
Final commit
marvenlee2486 May 29, 2022
3db27fd
Edit minor changes
marvenlee2486 May 29, 2022
eeadf9e
Final commit
marvenlee2486 May 29, 2022
cfd560c
return the error
marvenlee2486 May 29, 2022
b402de0
Revert
marvenlee2486 May 29, 2022
d43565b
Change to Upper Snake Case
marvenlee2486 May 29, 2022
a7f6405
Remove wRemove extra white space at the end of the code
marvenlee2486 May 29, 2022
41b0d0d
Remove wadd extra ace at the end of the code
marvenlee2486 May 29, 2022
87296fd
Modifed e2e tests
marvenlee2486 May 29, 2022
b59ac4d
Delete Bitwise e2e
marvenlee2486 May 29, 2022
3eb7175
Change e2e bitwise from select to values
marvenlee2486 May 29, 2022
90444b8
Revert "Change e2e bitwise from select to values"
marvenlee2486 May 30, 2022
fab8764
revision
lmatz May 30, 2022
14bb70e
Edit error
marvenlee2486 May 30, 2022
b8e3a13
Resolve all comments except 'no overflow' comment
marvenlee2486 Jun 1, 2022
78a54c3
Delete the additional test and solve formatting issues
marvenlee2486 Jun 1, 2022
5398d8a
Resolve test error
marvenlee2486 Jun 1, 2022
c66ca60
OTrucated to zero
marvenlee2486 Jun 1, 2022
a3c3cb3
Change error in test
marvenlee2486 Jun 1, 2022
ad7dda7
Change error in test
marvenlee2486 Jun 1, 2022
91095a3
Edit Test error
marvenlee2486 Jun 1, 2022
0b1cfb3
Edit test error
marvenlee2486 Jun 1, 2022
ec4c0e0
Change format
marvenlee2486 Jun 1, 2022
70a5d8e
Resolve compilation error
marvenlee2486 Jun 1, 2022
b773ad7
Resolve error
marvenlee2486 Jun 1, 2022
0d8e71f
Resolve error
marvenlee2486 Jun 1, 2022
53db214
Resolve test error
marvenlee2486 Jun 2, 2022
c87bbe0
Resolve test error
marvenlee2486 Jun 2, 2022
7e771f5
Resolve type_inference error
marvenlee2486 Jun 2, 2022
4a422c8
Resolve format error
marvenlee2486 Jun 2, 2022
1bc00ef
Resolve format error
marvenlee2486 Jun 2, 2022
0e68cee
Resolve format error
marvenlee2486 Jun 2, 2022
b7f0d46
Resolve test error
marvenlee2486 Jun 2, 2022
289d30c
Resolve error
marvenlee2486 Jun 2, 2022
05ba239
Resolve license fail
marvenlee2486 Jun 2, 2022
541c447
Edit the lose edition
marvenlee2486 Jun 2, 2022
5926d4e
Resolve error due to merge conflict resolve error
marvenlee2486 Jun 2, 2022
05be5d7
Revert to having overflow error
marvenlee2486 Jun 2, 2022
f3b9898
Merge branch 'main' into zongyu/bitwise_operation
marvenlee2486 Jun 2, 2022
45962dc
Remove autogenerated unwanted file
marvenlee2486 Jun 2, 2022
a157eb9
Resolve some comments - Duplication, Docs on Code
marvenlee2486 Jun 2, 2022
6f73499
Resolves all other comments
marvenlee2486 Jun 3, 2022
eb52467
Delete unwanted import
marvenlee2486 Jun 3, 2022
9ba0050
Merge branch 'main' into zongyu/bitwise_operation
marvenlee2486 Jun 3, 2022
30637e2
Resolve merge error
marvenlee2486 Jun 3, 2022
481e578
Format error
marvenlee2486 Jun 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions e2e_test/batch/bitwise.slt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
query I
values(3&5);
----
1

query I
values(3|5);
----
7

query I
values(3#5);
----
6

query I
values(1<<2);
----
4

query I
values(4>>2);
----
1

query I
values(~1);
----
-2
7 changes: 7 additions & 0 deletions proto/expr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ message ExprNode {
OR = 22;
NOT = 23;
IN = 24;
// bitwise operators
BITWISE_AND = 31;
BITWISE_OR = 32;
BITWISE_XOR = 33;
BITWISE_NOT = 34;
BITWISE_SHIFT_LEFT = 35;
BITWISE_SHIFT_RIGHT = 36;
// date functions
EXTRACT = 101;
TUMBLE_START = 103;
Expand Down
146 changes: 146 additions & 0 deletions src/expr/src/expr/expr_binary_nonnull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::expr::template::BinaryExpression;
use crate::expr::BoxedExpression;
use crate::for_all_cmp_variants;
use crate::vector_op::arithmetic_op::*;
use crate::vector_op::bitwise_op::*;
use crate::vector_op::cmp::*;
use crate::vector_op::extract::{extract_from_date, extract_from_timestamp};
use crate::vector_op::like::like_default;
Expand Down Expand Up @@ -102,6 +103,38 @@ macro_rules! gen_cmp_impl {
};
}

/// This macro helps create bitwise shift expression. The Output type is same as LHS of the
/// expression and the RHS of the expression is being match into u32. Similar to `gen_atm_impl`.
macro_rules! gen_shift_impl {
([$l:expr, $r:expr, $ret:expr], $( { $i1:ident, $i2:ident, $func:ident },)*) => {
match ($l.return_type(), $r.return_type()) {
$(
($i1! { type_match_pattern }, $i2! { type_match_pattern }) => {
Box::new(
BinaryExpression::<
$i1! { type_array },
$i2! { type_array },
$i1! { type_array },
_
>::new(
$l,
$r,
$ret,
$func::<
<$i1! { type_array } as Array>::OwnedItem,
<$i2! { type_array } as Array>::OwnedItem>,

)
)
},
)*
_ => {
unimplemented!("The expression ({:?}, {:?}, {:?}) using vectorized expression framework is not supported yet!", $l.return_type(), $r.return_type(), $ret)
}
}
};
}

/// Based on the data type of `$l`, `$r`, `$ret`, return corresponding expression struct with scalar
/// function inside.
/// * `$l`: left expression
Expand Down Expand Up @@ -206,6 +239,70 @@ macro_rules! gen_binary_expr_atm {
};
}

/// `gen_binary_expr_bitwise` is similar to `gen_binary_expr_atm`.
/// They are differentiate because bitwise operation does not support for float datatype.
marvenlee2486 marked this conversation as resolved.
Show resolved Hide resolved
/// * `$general_f`: generic atm function (require a common ``TryInto`` type for two input)
/// * `$i1`, `$i2`, `$rt`, `$func`: extra list passed to `$macro` directly
macro_rules! gen_binary_expr_bitwise {
(
$macro:ident,
$l:expr,
$r:expr,
$ret:expr,
$general_f:ident,
{
$( { $i1:ident, $i2:ident, $rt:ident, $func:ident }, )*
} $(,)?
) => {
$macro! {
[$l, $r, $ret],
{ int16, int16, int16, $general_f },
{ int16, int32, int32, $general_f },
{ int16, int64, int64, $general_f },
{ int32, int16, int32, $general_f },
{ int32, int32, int32, $general_f },
{ int32, int64, int64, $general_f },
{ int64, int16,int64, $general_f },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reformat it, cargo fmt --all seems not work on code in macro.

{ int64, int32,int64, $general_f },
{ int64, int64, int64, $general_f },
$(
{ $i1, $i2, $rt, $func },
)*
}
};
}

/// `gen_binary_expr_shift` is similar to `gen_binary_expr_bitwise`.
/// They are differentiate because shift operation have different typing rules.
/// * `$general_f`: generic atm function
/// `$rt` is not required because Type of the output is same as the Type of LHS of expression.
/// * `$i1`, `$i2`, `$func`: extra list passed to `$macro` directly
macro_rules! gen_binary_expr_shift {
(
$macro:ident,
$l:expr,
$r:expr,
$ret:expr,
$general_f:ident,
{
$( { $i1:ident, $i2:ident, $func:ident }, )*
} $(,)?
) => {
$macro! {
[$l, $r, $ret],
{ int16, int16, $general_f },
{ int32, int16, $general_f },
{ int16, int32, $general_f },
{ int32, int32, $general_f },
{ int64, int16, $general_f },
{ int64, int32, $general_f },
$(
{ $i1, $i2, $func },
)*
}
};
}

fn build_extract_expr(ret: DataType, l: BoxedExpression, r: BoxedExpression) -> BoxedExpression {
match r.return_type() {
DataType::Date => Box::new(
Expand Down Expand Up @@ -315,6 +412,54 @@ pub fn new_binary_expr(
},
}
}
// BitWise Operation
Type::BitwiseShiftLeft => {
gen_binary_expr_shift! {
gen_shift_impl,
l, r, ret,
general_shl,
{

},
}
}
Type::BitwiseShiftRight => {
gen_binary_expr_shift! {
gen_shift_impl,
l, r, ret,
general_shr,
{

},
}
}
Type::BitwiseAnd => {
gen_binary_expr_bitwise! {
gen_atm_impl,
l, r, ret,
general_bitand,
{
},
}
}
Type::BitwiseOr => {
gen_binary_expr_bitwise! {
gen_atm_impl,
l, r, ret,
general_bitor,
{
},
}
}
Type::BitwiseXor => {
gen_binary_expr_bitwise! {
gen_atm_impl,
l, r, ret,
general_bitxor,
{
},
}
}
Type::Extract => build_extract_expr(ret, l, r),
Type::RoundDigit => Box::new(
BinaryExpression::<DecimalArray, I32Array, DecimalArray, _>::new(
Expand All @@ -328,6 +473,7 @@ pub fn new_binary_expr(
l, r, ret, position,
)),
Type::TumbleStart => new_tumble_start(l, r, ret),

tp => {
unimplemented!(
"The expression {:?} using vectorized expression framework is not supported yet!",
Expand Down
10 changes: 10 additions & 0 deletions src/expr/src/expr/expr_unary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::expr::template::UnaryNullableExpression;
use crate::expr::BoxedExpression;
use crate::vector_op::arithmetic_op::{decimal_abs, general_abs, general_neg};
use crate::vector_op::ascii::ascii;
use crate::vector_op::bitwise_op::general_bitnot;
use crate::vector_op::cast::*;
use crate::vector_op::cmp::{is_false, is_not_false, is_not_true, is_true};
use crate::vector_op::conjunction;
Expand Down Expand Up @@ -302,6 +303,15 @@ pub fn new_unary_expr(
}
}
}
(ProstType::BitwiseNot, _, _) => {
gen_unary_impl! {
[ "BitwiseNot", child_expr, return_type],
{ int16, int16, general_bitnot },
{ int32, int32, general_bitnot },
{ int64, int64, general_bitnot },

}
}
(ProstType::Ceil, _, _) => {
gen_round_expr! {"Ceil", child_expr, return_type, ceil_f64, ceil_decimal}
}
Expand Down
8 changes: 6 additions & 2 deletions src/expr/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,14 @@ pub fn build_from_prost(prost: &ExprNode) -> Result<BoxedExpression> {

match prost.get_expr_type()? {
Cast | Upper | Lower | Md5 | Not | IsTrue | IsNotTrue | IsFalse | IsNotFalse | IsNull
| IsNotNull | Neg | Ascii | Abs | Ceil | Floor | Round => build_unary_expr_prost(prost),
| IsNotNull | Neg | Ascii | Abs | Ceil | Floor | Round | BitwiseNot => {
build_unary_expr_prost(prost)
}
Equal | NotEqual | LessThan | LessThanOrEqual | GreaterThan | GreaterThanOrEqual | Add
| Subtract | Multiply | Divide | Modulus | Extract | RoundDigit | TumbleStart
| Position => build_binary_expr_prost(prost),
| Position | BitwiseShiftLeft | BitwiseShiftRight | BitwiseAnd | BitwiseOr | BitwiseXor => {
build_binary_expr_prost(prost)
}
And | Or | IsDistinctFrom => build_nullable_binary_expr_prost(prost),
ToChar => build_to_char_expr(prost),
Coalesce => CoalesceExpression::try_from(prost).map(Expression::boxed),
Expand Down
103 changes: 103 additions & 0 deletions src/expr/src/vector_op/bitwise_op.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright 2022 Singularity Data
//
// Licensed 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.
use std::any::type_name;
use std::convert::TryInto;
use std::fmt::Debug;
use std::ops::{BitAnd, BitOr, BitXor, Not};

use num_traits::{CheckedShl, CheckedShr};
use risingwave_common::error::ErrorCode::{InternalError, NumericValueOutOfRange};
use risingwave_common::error::{Result, RwError};

use crate::vector_op::arithmetic_op::general_atm;

// Conscious decision for shl and shr is made here to diverge from PostgreSQL.
// If overflow happens, instead of truncated to zero, we return overflow error as this is unexpected
// behaviour If the RHS is negative, instead of having an unexpected answer, we return an error.
marvenlee2486 marked this conversation as resolved.
Show resolved Hide resolved
#[inline(always)]
pub fn general_shl<T1, T2>(l: T1, r: T2) -> Result<T1>
where
T1: CheckedShl + Debug,
T2: TryInto<u32> + Debug,
{
general_shift(l, r, |a, b| match a.checked_shl(b) {
Some(c) => Ok(c),
None => Err(RwError::from(NumericValueOutOfRange)),
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI a.checked_shl(b).ok_or_else
Not sure if this looks better or worse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I change it for mine part. general_shift(l, r, |a,b| a.checked_shl(b).ok_or_else( || RwError::from(NumericValueOutOfRange))). I feel it looks better.

Just to point out, the previous writing I was copying from arithmetic_op.rs. Should it be changed as well for consistency across similar code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping them consistent would be better.

}

#[inline(always)]
pub fn general_shr<T1, T2>(l: T1, r: T2) -> Result<T1>
where
T1: CheckedShr + Debug,
T2: TryInto<u32> + Debug,
{
general_shift(l, r, |a, b| match a.checked_shr(b) {
Some(c) => Ok(c),
None => Err(RwError::from(NumericValueOutOfRange)),
})
}

#[inline(always)]
pub fn general_shift<T1, T2, F>(l: T1, r: T2, atm: F) -> Result<T1>
where
T1: Debug,
T2: TryInto<u32> + Debug,
F: FnOnce(T1, u32) -> Result<T1>,
{
// TODO: We need to improve the error message
let r: u32 = r.try_into().map_err(|_| {
RwError::from(InternalError(format!(
"Can't convert {} to {}",
type_name::<T2>(),
type_name::<u32>()
)))
})?;
atm(l, r)
}

#[inline(always)]
pub fn general_bitand<T1, T2, T3>(l: T1, r: T2) -> Result<T3>
where
T1: TryInto<T3> + Debug,
T2: TryInto<T3> + Debug,
T3: BitAnd<Output = T3>,
{
general_atm(l, r, |a, b| Ok(a.bitand(b)))
}

#[inline(always)]
pub fn general_bitor<T1, T2, T3>(l: T1, r: T2) -> Result<T3>
where
T1: TryInto<T3> + Debug,
T2: TryInto<T3> + Debug,
T3: BitOr<Output = T3>,
{
general_atm(l, r, |a, b| Ok(a.bitor(b)))
}

#[inline(always)]
pub fn general_bitxor<T1, T2, T3>(l: T1, r: T2) -> Result<T3>
where
T1: TryInto<T3> + Debug,
T2: TryInto<T3> + Debug,
T3: BitXor<Output = T3>,
{
general_atm(l, r, |a, b| Ok(a.bitxor(b)))
}

#[inline(always)]
pub fn general_bitnot<T1: Not<Output = T1>>(expr: T1) -> Result<T1> {
Ok(expr.not())
}
1 change: 1 addition & 0 deletions src/expr/src/vector_op/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
pub mod agg;
pub mod arithmetic_op;
pub mod ascii;
pub mod bitwise_op;
pub mod cast;
pub mod cmp;
pub mod conjunction;
Expand Down
Loading