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.

Sign up for GitHub

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

minor: Refactor binary expr serde to reduce code duplication #1053

Merged
merged 5 commits into from
Nov 5, 2024
Merged

minor: Refactor binary expr serde to reduce code duplication #1053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1240af0
Use one BinaryExpr definition in protobuf
andygrove Nov 4, 2024
ee90401
refactor And
andygrove Nov 4, 2024
cb55b00
refactor remaining binary expressions
andygrove Nov 4, 2024
d6293e1
update test
andygrove Nov 4, 2024
547d8d8
update test
andygrove Nov 4, 2024
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
2 changes: 1 addition & 1 deletion native/core/src/execution/datafusion/planner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2388,7 +2388,7 @@ mod tests {
};

let expr = spark_expression::Expr {
expr_struct: Some(Eq(Box::new(spark_expression::Equal {
expr_struct: Some(Eq(Box::new(spark_expression::BinaryExpr {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is the only place where the expression needed to be updated explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

left: Some(Box::new(left)),
right: Some(Box::new(right)),
}))),
Expand Down
137 changes: 21 additions & 116 deletions native/proto/src/proto/expr.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,41 @@ message Expr {
Multiply multiply = 6;
Divide divide = 7;
Cast cast = 8;
Equal eq = 9;
NotEqual neq = 10;
GreaterThan gt = 11;
GreaterThanEqual gt_eq = 12;
LessThan lt = 13;
LessThanEqual lt_eq = 14;
BinaryExpr eq = 9;
BinaryExpr neq = 10;
BinaryExpr gt = 11;
BinaryExpr gt_eq = 12;
BinaryExpr lt = 13;
BinaryExpr lt_eq = 14;
IsNull is_null = 15;
IsNotNull is_not_null = 16;
And and = 17;
Or or = 18;
BinaryExpr and = 17;
BinaryExpr or = 18;
SortOrder sort_order = 19;
Substring substring = 20;
StringSpace string_space = 21;
Hour hour = 22;
Minute minute = 23;
Second second = 24;
CheckOverflow check_overflow = 25;
Like like = 26;
StartsWith startsWith = 27;
EndsWith endsWith = 28;
Contains contains = 29;
RLike rlike = 30;
BinaryExpr like = 26;
BinaryExpr startsWith = 27;
BinaryExpr endsWith = 28;
BinaryExpr contains = 29;
BinaryExpr rlike = 30;
ScalarFunc scalarFunc = 31;
EqualNullSafe eqNullSafe = 32;
NotEqualNullSafe neqNullSafe = 33;
BitwiseAnd bitwiseAnd = 34;
BitwiseOr bitwiseOr = 35;
BitwiseXor bitwiseXor = 36;
BinaryExpr eqNullSafe = 32;
BinaryExpr neqNullSafe = 33;
BinaryExpr bitwiseAnd = 34;
BinaryExpr bitwiseOr = 35;
BinaryExpr bitwiseXor = 36;
Remainder remainder = 37;
CaseWhen caseWhen = 38;
In in = 39;
Not not = 40;
UnaryMinus unary_minus = 41;
BitwiseShiftRight bitwiseShiftRight = 42;
BitwiseShiftLeft bitwiseShiftLeft = 43;
BinaryExpr bitwiseShiftRight = 42;
BinaryExpr bitwiseShiftLeft = 43;
IfExpr if = 44;
NormalizeNaNAndZero normalize_nan_and_zero = 45;
TruncDate truncDate = 46;
Expand Down Expand Up @@ -269,52 +269,7 @@ message Cast {
bool allow_incompat = 5;
}

message Equal {
Expr left = 1;
Expr right = 2;
}

message NotEqual {
Expr left = 1;
Expr right = 2;
}

message EqualNullSafe {
Expr left = 1;
Expr right = 2;
}

message NotEqualNullSafe {
Expr left = 1;
Expr right = 2;
}

message GreaterThan {
Expr left = 1;
Expr right = 2;
}

message GreaterThanEqual {
Expr left = 1;
Expr right = 2;
}

message LessThan {
Expr left = 1;
Expr right = 2;
}

message LessThanEqual {
Expr left = 1;
Expr right = 2;
}

message And {
Expr left = 1;
Expr right = 2;
}

message Or {
message BinaryExpr {
Expr left = 1;
Expr right = 2;
}
Expand Down Expand Up @@ -384,62 +339,12 @@ message CheckOverflow {
bool fail_on_error = 3;
}

message Like {
Expr left = 1;
Expr right = 2;
}

message RLike {
Expr left = 1;
Expr right = 2;
}

message StartsWith {
Expr left = 1;
Expr right = 2;
}

message EndsWith {
Expr left = 1;
Expr right = 2;
}

message Contains {
Expr left = 1;
Expr right = 2;
}

message ScalarFunc {
string func = 1;
repeated Expr args = 2;
DataType return_type = 3;
}

message BitwiseAnd {
Expr left = 1;
Expr right = 2;
}

message BitwiseOr {
Expr left = 1;
Expr right = 2;
}

message BitwiseXor {
Expr left = 1;
Expr right = 2;
}

message BitwiseShiftRight {
Expr left = 1;
Expr right = 2;
}

message BitwiseShiftLeft {
Expr left = 1;
Expr right = 2;
}

message CaseWhen {
// The expr field is added to be consistent with CaseExpr definition in DataFusion.
// This field is not really used. When constructing a CaseExpr, this expr field
Expand Down
Loading
Loading