DeviousCardi commented on code in PR #6732:
URL: https://github.com/apache/datafusion-comet/pull/6732#discussion_r4214285723


##########
spark/src/test/scala/org/apache/comet/exec/CometAggregateSuite.scala:
##########
@@ -3297,20 +3297,38 @@ class CometAggregateSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
     (1 to 50).flatMap(_ => Seq((maxDec38_0, 1)))
   }
 
+  /**
+   * Spark's integral SUM adds through `Add`, so an ANSI overflow is `long 
overflow` with the
+   * `try_add` suggestion. Compare the structured error, not just its error 
class.
+   */
+  private def assertAnsiSumOverflowMatchesSpark(df: DataFrame): Unit = {
+    val (sparkError, cometError) = checkSparkAnswerMaybeThrows(df)
+    def structured(error: Option[Throwable]): SparkThrowable with Throwable = {
+      val failure = error.getOrElse(fail("Expected SUM overflow in ANSI mode"))
+      causeChain(failure)
+        .collect { case e: SparkThrowable with Throwable => e }
+        .lastOption
+        .getOrElse(fail(s"Expected SparkThrowable: $failure"))
+    }
+    val expected = structured(sparkError)
+    val actual = structured(cometError)
+    assert(expected.getErrorClass == "ARITHMETIC_OVERFLOW")
+    assert(actual.getClass == expected.getClass)
+    assert(actual.getErrorClass == expected.getErrorClass)
+    assert(actual.getSqlState == expected.getSqlState)
+    assert(actual.getMessageParameters == expected.getMessageParameters)
+    assert(actual.getMessage.contains("long overflow"))

Review Comment:
   Thanks, you're right. Removed the literal `long overflow` assertion in 
3ab150d. The `getMessageParameters` comparison against Spark already covers the 
wording on every profile, including 4.2's normalized `overflow`. I kept the 
`try_add` check because the suggestion is present on all supported versions. I 
also updated the helper's doc comment to say why the wording isn't asserted 
directly.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to