apache / datafusion-comet

Apache DataFusion Comet Spark Accelerator
https://datafusion.apache.org/comet
Apache License 2.0
823 stars 163 forks source link

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

Closed andygrove closed 2 weeks ago

andygrove commented 2 weeks ago

Which issue does this PR close?

N/A

Rationale for this change

We currently have 20 duplicate structs defined in protobuf for binary expressions, such as:

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

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

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

As a result, we have a lot of duplicate code in QueryPlanSerde for serializing binary expressions that cannot easily be refactored because each code block references a different (but identical) generated class.

What changes are included in this PR?

How are these changes tested?

Existing tests

andygrove commented 2 weeks ago

I will follow up with similar PRs for UnaryExpr and MathExpr

andygrove commented 2 weeks ago

Thanks for the reviews @kazuyukitanimura @parthchandra @parthchandra