andygrove opened a new issue, #2019:
URL: https://github.com/apache/datafusion-comet/issues/2019

   ### What is the problem the feature request solves?
   
   The `QueryPlanSerde.exprToProtoInternal` method contains logic for 
serializing Spark expressions to protocol buffer format and also contains 
checks that Comet supports the expression. This file has grown very large and 
is hard to navigate, so we would like to refactor this logic such that the 
per-expression logic is moved into separate classes.
   
   As an example, here is the original approach for handling the `Add` 
expression:
   
   ```scala
         case add @ Add(left, right, _) if supportedDataType(left.dataType) =>
           createMathExpression(
             expr,
             left,
             right,
             inputs,
             binding,
             add.dataType,
             add.evalMode == EvalMode.ANSI,
             (builder, mathExpr) => builder.setAdd(mathExpr))
   
         case add @ Add(left, _, _) if !supportedDataType(left.dataType) =>
           withInfo(add, s"Unsupported datatype ${left.dataType}")
           None
   ```
   
   The new approach is to move this into a separate file and class:
   
   ```scala
   object CometAdd extends CometExpressionSerde with MathBase {
     override def convert(
         expr: Expression,
         inputs: Seq[Attribute],
         binding: Boolean): Option[ExprOuterClass.Expr] = {
       val add = expr.asInstanceOf[Add]
       if (!supportedDataType(add.left.dataType)) {
         withInfo(add, s"Unsupported datatype ${add.left.dataType}")
         return None
       }
       createMathExpression(
         expr,
         add.left,
         add.right,
         inputs,
         binding,
         add.dataType,
         add.evalMode == EvalMode.ANSI,
         (builder, mathExpr) => builder.setAdd(mathExpr))
     }
   }
   ```
   
   These classes are then referenced from QueryPlanSerde in a map:
   
   ```scala
     private val exprSerdeMap: Map[Class[_], CometExpressionSerde] = Map(
       classOf[Add] -> CometAdd,
       classOf[Subtract] -> CometSubtract,
       classOf[Multiply] -> CometMultiply,
       ...
   ```
   
   ### Describe the potential solution
   
   _No response_
   
   ### Additional context
   
   _No response_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to