mbutrovich commented on code in PR #960:
URL: https://github.com/apache/datafusion-comet/pull/960#discussion_r1773845063


##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -873,12 +873,18 @@ class CometSparkSessionExtensions
           LessThan(normalizeNaNAndZero(left), normalizeNaNAndZero(right))
         case LessThanOrEqual(left, right) =>
           LessThanOrEqual(normalizeNaNAndZero(left), 
normalizeNaNAndZero(right))
+        case Divide(left, right, evalMode) =>
+          Divide(left, normalizeNaNAndZero(right), evalMode)
+        case Remainder(left, right, evalMode) =>
+          Remainder(left, normalizeNaNAndZero(right), evalMode)
       }
     }
 
     def normalizeNaNAndZero(expr: Expression): Expression = {
       expr match {
         case _: KnownFloatingPointNormalized => expr
+        case FloatLiteral(f) if !f.equals(-0.0f) => expr

Review Comment:
   Just clarifying: this is saying that as long as the literal isn't -0.0, we 
don't have to normalize it? And it doesn't look like there's any way to define 
a literal NaN, so we don't handle that case?



##########
docs/source/contributor-guide/development.md:
##########
@@ -91,13 +91,15 @@ Comet has a plan stability testing framework that can be 
used to test the stabil
 The plan stability testing framework is located in the `spark` module and can 
be run using the following command:
 
 ```sh
-./mvnw -pl spark 
-Dsuites="org.apache.spark.sql.comet.CometTPCDSV1_4_PlanStabilitySuite" test
+./mvnw -pl spark 
-Dsuites="org.apache.spark.sql.comet.CometTPCDSV1_4_PlanStabilitySuite" -nsu 
test
+./mvnw -pl spark 
-Dsuites="org.apache.spark.sql.comet.CometTPCDSV1_4_PlanStabilitySuite" 
-Pspark-3.5 -nsu test

Review Comment:
   Since you added these steps to the testing commands, does it makes sense to 
duplicate them below for the generation commands?



-- 
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

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