[GitHub] [spark] rednaxelafx commented on a change in pull request #25080: [SPARK-28306][SQL] Make NormalizeFloatingNumbers rule idempotent

[GitBox <gi...@apache.org>]

rednaxelafx commented on a change in pull request #25080: [SPARK-28306][SQL] Make NormalizeFloatingNumbers rule idempotent
URL: https://github.com/apache/spark/pull/25080#discussion_r301819519
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFloatingNumbers.scala
 ##########
 @@ -116,7 +124,7 @@ object NormalizeFloatingNumbers extends Rule[LogicalPlan] {
       CreateMap(children.map(normalize))
 
     case _ if expr.dataType == FloatType || expr.dataType == DoubleType =>
-      NormalizeNaNAndZero(expr)
+      KnownFloatingPointNormalized(NormalizeNaNAndZero(expr))
 
 Review comment:
   This has a much less impact than the `AnalysisBarrier` -- this only applies to expressions whereas the `AnalysisBarrier` applied to plans.
   We'd to leave markers in place in case a plan gets re-optimized after the initial optimization, and we have to have something that provides such information persisted in the plan.
   
   The alternative for providing this information would be something like having a new dedicated expression type for floating point array normalization, which would also be disruptive to the expression tree structure. In terms of code reuse and semantic clarity, I'd say Yesheng's current design strikes the best balance.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org