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