You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/06/08 21:34:10 UTC

[GitHub] [spark] shahidki31 commented on a change in pull request #32659: [SPARK-22639][SQL] Support aggregate cbo stats estimation if the group by clause involves substring

shahidki31 commented on a change in pull request #32659:
URL: https://github.com/apache/spark/pull/32659#discussion_r647808937



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala
##########
@@ -80,6 +80,54 @@ object EstimationUtils {
     expressions.collect {
       case alias @ Alias(attr: Attribute, _) if attributeStats.contains(attr) =>
         alias.toAttribute -> attributeStats(attr)
+      case alias @ Alias(expn: Expression, _) if isExpressionStatsExist(expn, attributeStats) =>

Review comment:
       Yes, We need to update for project node as well. If the grouping key is aliased, or if the grouping key is complex expression, the expressions are pushed to project node.
   Even if the aggregate node is not there, still we need to update the stats for expressions in the project node.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala
##########
@@ -80,6 +80,54 @@ object EstimationUtils {
     expressions.collect {
       case alias @ Alias(attr: Attribute, _) if attributeStats.contains(attr) =>
         alias.toAttribute -> attributeStats(attr)
+      case alias @ Alias(expn: Expression, _) if isExpressionStatsExist(expn, attributeStats) =>
+        getExpressionStats(alias.toAttribute, expn, attributeStats)
+    }
+  }
+
+  // Support for substring expressions.
+  // TODO: Support for more expressions like Multiply.

Review comment:
       For substring, yes we can pass the child plan stats. But for other expressions it might not be true. For eg: Literal, Concat string etc. We might need to find more expressions if we can estimate stats. For eg: Multiply.




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



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