minor: Refactor binary expr serde to reduce code duplication

[andygrove]

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?

• Use a single definition of BinaryExpr
• Refactor QueryPlanSerde to remove a lot of duplicate boilerplate code

How are these changes tested?

Existing tests

[mbutrovich]

This is a beautiful refactor!

[andygrove]

I will follow up with similar PRs for UnaryExpr and MathExpr

[parthchandra]

LGTM.

[parthchandra]

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

[andygrove]

Yes

[kazuyukitanimura]

I feel these lines can be a part of createBinaryExpr so that we do not have to map()?

[andygrove]

This block of code is specifically for creating BinaryExpr eq = 9 (note the call to setEq that uses the result of createBinaryExpr). Each of these 20 similar blocks of code is for setting a specific top-level expr_struct. It is only the contained BinaryExpr struct that is common to each one. Does that make sense?

[andygrove]

Hmm .. perhaps I could refactor farther so that the map receives an ExprOuterClass.Expr.Builder and then only has to call setEq in the map block. That would be more concise. I will try that as a follow on PR after #1056 is merged

[andygrove]

Thanks for the reviews @kazuyukitanimura @parthchandra @parthchandra