maropu commented on a change in pull request #32494:
URL: https://github.com/apache/spark/pull/32494#discussion_r634992199



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/UnionEstimation.scala
##########
@@ -70,8 +68,30 @@ object UnionEstimation {
       None
     }
 
-    val unionOutput = union.output
+    val newMinMaxStats = computeMinMaxStats(union)
+    val newNullCountStats = computeNullCountStats(union)
+    val newAttrStats = {
+      val updatedNullCountStats = newNullCountStats.keys.map { key =>

Review comment:
       This is my last comment; in this access pattern, It seems we don't need 
to convert the null stats into `AttributeMap` in `computeNullCountStats`. How 
about rewriting it so that they return `Seq[Atttribute, ColumnStat]`? Then, we 
can create a newAttrStats like this;
   ```
      val newAttrStats = {
         val baseStats = AttributeMap(newMinMaxStats)
         val overwriteStats = newNullCountStats.map { case attrStat @ (attr, 
stat) =>
           baseStats.get(attr).map { baseStat =>
             attr -> baseStat.copy(nullCount = stat.nullCount)
           }.getOrElse(attrStat)
         }
         AttributeMap(newMinMaxStats ++ overwriteStats)
       }
   ```
   See[ the 
commit](https://github.com/apache/spark/commit/bdba39fa649c0219c9e4c01bbab06065b03de7c8)
 for more details.




-- 
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:
us...@infra.apache.org



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

Reply via email to