Disable abs and signum

apache · Jul 19, 2024 · 782ee35 · 782ee35
1 parent e8765d4
commit 782ee35
Showing 1 changed file with 25 additions and 15 deletions.
diff --git a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala
@@ -1615,18 +1615,26 @@ object QueryPlanSerde extends Logging with ShimQueryPlanSerde with CometExprShim
             None
           }
 
-        case Abs(child, failOnErr) =>
-          val childExpr = exprToProtoInternal(child, inputs)
-          if (childExpr.isDefined) {
-            val evalModeStr =
-              if (failOnErr) ExprOuterClass.EvalMode.ANSI else ExprOuterClass.EvalMode.LEGACY
-            val absBuilder = ExprOuterClass.Abs.newBuilder()
-            absBuilder.setChild(childExpr.get)
-            absBuilder.setEvalMode(evalModeStr)
-            Some(Expr.newBuilder().setAbs(absBuilder).build())
-          } else {
-            withInfo(expr, child)
+        case abs @ Abs(child, failOnErr) =>
+          if (!failOnErr) {
+            // abs in legacy mode is not implemented correctly and will
+            // return incorrect results in some cases
+            // https://github.com/apache/datafusion-comet/issues/666
+            withInfo(abs, "Abs in legacy mode is not supported")
             None
+          } else {
+            val childExpr = exprToProtoInternal(child, inputs)
+            if (childExpr.isDefined) {
+              val evalModeStr =
+                if (failOnErr) ExprOuterClass.EvalMode.ANSI else ExprOuterClass.EvalMode.LEGACY
+              val absBuilder = ExprOuterClass.Abs.newBuilder()
+              absBuilder.setChild(childExpr.get)
+              absBuilder.setEvalMode(evalModeStr)
+              Some(Expr.newBuilder().setAbs(absBuilder).build())
+            } else {
+              withInfo(expr, child)
+              None
+            }
           }
 
         case Acos(child) =>
@@ -1766,10 +1774,12 @@ object QueryPlanSerde extends Logging with ShimQueryPlanSerde with CometExprShim
               optExprWithInfo(optExpr, expr, r.child)
           }
 
-        case Signum(child) =>
-          val childExpr = exprToProtoInternal(child, inputs)
-          val optExpr = scalarExprToProto("signum", childExpr)
-          optExprWithInfo(optExpr, expr, child)
+        // TODO enable once https://github.com/apache/datafusion/issues/11557 is fixed or
+        // when we have a Spark-compatible version implemented in Comet
+//        case Signum(child) =>
+//          val childExpr = exprToProtoInternal(child, inputs)
+//          val optExpr = scalarExprToProto("signum", childExpr)
+//          optExprWithInfo(optExpr, expr, child)
 
         case Sin(child) =>
           val childExpr = exprToProtoInternal(child, inputs)