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 2020/09/21 10:53:38 UTC

[GitHub] [spark] peter-toth opened a new pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

peter-toth opened a new pull request #29816:
URL: https://github.com/apache/spark/pull/29816


   ### What changes were proposed in this pull request?
   This PR adds foldable propagation from `Aggregate` as per: https://github.com/apache/spark/pull/29771#discussion_r490412031
   
   ### Why are the changes needed?
   This is an improvement, `Aggregate`'s `aggregateExpressions` can contain foldables that can be propagated up.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   New UT.
   


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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696044079






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


[GitHub] [spark] SparkQA commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696356721


   **[Test build #128943 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128943/testReport)** for PR 29816 at commit [`474c7c0`](https://github.com/apache/spark/commit/474c7c03e4409f41e6792f7f5d13fd61775689c5).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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


[GitHub] [spark] SparkQA commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696210891


   **[Test build #128943 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128943/testReport)** for PR 29816 at commit [`474c7c0`](https://github.com/apache/spark/commit/474c7c03e4409f41e6792f7f5d13fd61775689c5).


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


[GitHub] [spark] cloud-fan commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492179483



##########
File path: sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q41.sf100/explain.txt
##########
@@ -73,19 +73,19 @@ Input [2]: [i_manufact#2, count#9]
 Keys [1]: [i_manufact#2]
 Functions [1]: [count(1)]
 Aggregate Attributes [1]: [count(1)#11]
-Results [3]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13, true AS alwaysTrue#14]
+Results [2]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13]

Review comment:
       This file is a positive change.




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


[GitHub] [spark] peter-toth commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492167457



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##########
@@ -703,14 +708,20 @@ object FoldablePropagation extends Rule[LogicalPlan] {
     }
   }
 
+  private def collectFoldables(expressions: Seq[NamedExpression]) = {
+    AttributeMap(expressions.collect {
+      case a: Alias if a.child.foldable => (a.toAttribute, a)
+    })
+  }
+
   /**
    * List of all [[UnaryNode]]s which allow foldable propagation.
    */
   private def canPropagateFoldables(u: UnaryNode): Boolean = u match {
     // Handling `Project` is moved to `propagateFoldables`.
     case _: Filter => true
     case _: SubqueryAlias => true
-    case _: Aggregate => true
+    // Handling `Aggregate` is moved to `propagateFoldables`.

Review comment:
       Ok, will change it.




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


[GitHub] [spark] viirya commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696220156


   Please remove [WIP] before merging.


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


[GitHub] [spark] viirya commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696220156


   Please remove [WIP] before merging.


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


[GitHub] [spark] dongjoon-hyun closed pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #29816:
URL: https://github.com/apache/spark/pull/29816


   


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


[GitHub] [spark] cloud-fan commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696207235


   you can just regenerate the golden file and push the change. Then the reviewers can look at the plan change and approve it if it's fine.


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


[GitHub] [spark] peter-toth commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696273137


   > Please remove [WIP] before merging.
   
   Done, thanks.


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


[GitHub] [spark] SparkQA removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696210891


   **[Test build #128943 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128943/testReport)** for PR 29816 at commit [`474c7c0`](https://github.com/apache/spark/commit/474c7c03e4409f41e6792f7f5d13fd61775689c5).


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


[GitHub] [spark] AmplabJenkins commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696211670






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


[GitHub] [spark] dongjoon-hyun closed pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #29816:
URL: https://github.com/apache/spark/pull/29816


   


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


[GitHub] [spark] AmplabJenkins commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696176680






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


[GitHub] [spark] AmplabJenkins commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696044079






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


[GitHub] [spark] peter-toth commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696210890


   > you can just regenerate the golden file and push the change. Then the reviewers can look at the plan change and approve it if it's fine.
   
   Thanks. I did it already but haven't had time to check the changes. Anyway, added it as a new commit.


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


[GitHub] [spark] cloud-fan commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696207235


   you can just regenerate the golden file and push the change. Then the reviewers can look at the plan change and approve it if it's fine.


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


[GitHub] [spark] SparkQA removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696043396






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


[GitHub] [spark] peter-toth commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696041188


   cc @cloud-fan, @dongjoon-hyun, @gatorsmile, @maropu, @rednaxelafx, @viirya 


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


[GitHub] [spark] AmplabJenkins commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696357833






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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696044079






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


[GitHub] [spark] peter-toth commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696041188






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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696357833






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


[GitHub] [spark] SparkQA commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696043396






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


[GitHub] [spark] maropu commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-697067845


   late LGTM and thanks for the update, @peter-toth !


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


[GitHub] [spark] peter-toth commented on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696204480


   > LGTM if tests can pass
   
   I need to look into plan stability failures first.


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


[GitHub] [spark] AmplabJenkins commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696044079






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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696176694


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128933/
   Test FAILed.


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


[GitHub] [spark] maropu commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-697067845


   late LGTM and thanks for the update, @peter-toth !


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


[GitHub] [spark] SparkQA commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696043396


   **[Test build #128933 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128933/testReport)** for PR 29816 at commit [`b6276c9`](https://github.com/apache/spark/commit/b6276c9c1c8c623d0c173c2d8c99701dd8a82a70).


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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696176680


   Merged build finished. Test FAILed.


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


[GitHub] [spark] SparkQA commented on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696175627


   **[Test build #128933 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128933/testReport)** for PR 29816 at commit [`b6276c9`](https://github.com/apache/spark/commit/b6276c9c1c8c623d0c173c2d8c99701dd8a82a70).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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


[GitHub] [spark] cloud-fan commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492164247



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##########
@@ -703,14 +708,20 @@ object FoldablePropagation extends Rule[LogicalPlan] {
     }
   }
 
+  private def collectFoldables(expressions: Seq[NamedExpression]) = {
+    AttributeMap(expressions.collect {
+      case a: Alias if a.child.foldable => (a.toAttribute, a)
+    })
+  }
+
   /**
    * List of all [[UnaryNode]]s which allow foldable propagation.
    */
   private def canPropagateFoldables(u: UnaryNode): Boolean = u match {
     // Handling `Project` is moved to `propagateFoldables`.
     case _: Filter => true
     case _: SubqueryAlias => true
-    case _: Aggregate => true
+    // Handling `Aggregate` is moved to `propagateFoldables`.

Review comment:
       can we merge this comment with ```// Handling `Project` is moved to `propagateFoldables`.```?

##########
File path: sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q41.sf100/explain.txt
##########
@@ -73,19 +73,19 @@ Input [2]: [i_manufact#2, count#9]
 Keys [1]: [i_manufact#2]
 Functions [1]: [count(1)]
 Aggregate Attributes [1]: [count(1)#11]
-Results [3]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13, true AS alwaysTrue#14]
+Results [2]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13]

Review comment:
       This file is a positive change.




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


[GitHub] [spark] SparkQA removed a comment on pull request #29816: [SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696043396


   **[Test build #128933 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128933/testReport)** for PR 29816 at commit [`b6276c9`](https://github.com/apache/spark/commit/b6276c9c1c8c623d0c173c2d8c99701dd8a82a70).


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


[GitHub] [spark] cloud-fan commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492164247



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##########
@@ -703,14 +708,20 @@ object FoldablePropagation extends Rule[LogicalPlan] {
     }
   }
 
+  private def collectFoldables(expressions: Seq[NamedExpression]) = {
+    AttributeMap(expressions.collect {
+      case a: Alias if a.child.foldable => (a.toAttribute, a)
+    })
+  }
+
   /**
    * List of all [[UnaryNode]]s which allow foldable propagation.
    */
   private def canPropagateFoldables(u: UnaryNode): Boolean = u match {
     // Handling `Project` is moved to `propagateFoldables`.
     case _: Filter => true
     case _: SubqueryAlias => true
-    case _: Aggregate => true
+    // Handling `Aggregate` is moved to `propagateFoldables`.

Review comment:
       can we merge this comment with ```// Handling `Project` is moved to `propagateFoldables`.```?




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


[GitHub] [spark] viirya commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492184213



##########
File path: sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q41.sf100/explain.txt
##########
@@ -73,19 +73,19 @@ Input [2]: [i_manufact#2, count#9]
 Keys [1]: [i_manufact#2]
 Functions [1]: [count(1)]
 Aggregate Attributes [1]: [count(1)#11]
-Results [3]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13, true AS alwaysTrue#14]
+Results [2]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13]
 
 (12) Filter [codegen id : 2]
-Input [3]: [item_cnt#12, i_manufact#2#13, alwaysTrue#14]
-Condition : (if (isnull(alwaysTrue#14)) 0 else item_cnt#12 > 0)
+Input [2]: [item_cnt#12, i_manufact#2#13]
+Condition : (item_cnt#12 > 0)
 
 (13) Project [codegen id : 2]
 Output [1]: [i_manufact#2#13]
-Input [3]: [item_cnt#12, i_manufact#2#13, alwaysTrue#14]
+Input [2]: [item_cnt#12, i_manufact#2#13]

Review comment:
       this looks good.




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


[GitHub] [spark] viirya commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492184213



##########
File path: sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q41.sf100/explain.txt
##########
@@ -73,19 +73,19 @@ Input [2]: [i_manufact#2, count#9]
 Keys [1]: [i_manufact#2]
 Functions [1]: [count(1)]
 Aggregate Attributes [1]: [count(1)#11]
-Results [3]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13, true AS alwaysTrue#14]
+Results [2]: [count(1)#11 AS item_cnt#12, i_manufact#2 AS i_manufact#2#13]
 
 (12) Filter [codegen id : 2]
-Input [3]: [item_cnt#12, i_manufact#2#13, alwaysTrue#14]
-Condition : (if (isnull(alwaysTrue#14)) 0 else item_cnt#12 > 0)
+Input [2]: [item_cnt#12, i_manufact#2#13]
+Condition : (item_cnt#12 > 0)
 
 (13) Project [codegen id : 2]
 Output [1]: [i_manufact#2#13]
-Input [3]: [item_cnt#12, i_manufact#2#13, alwaysTrue#14]
+Input [2]: [item_cnt#12, i_manufact#2#13]

Review comment:
       this looks good.




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


[GitHub] [spark] peter-toth commented on a change in pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #29816:
URL: https://github.com/apache/spark/pull/29816#discussion_r492167457



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##########
@@ -703,14 +708,20 @@ object FoldablePropagation extends Rule[LogicalPlan] {
     }
   }
 
+  private def collectFoldables(expressions: Seq[NamedExpression]) = {
+    AttributeMap(expressions.collect {
+      case a: Alias if a.child.foldable => (a.toAttribute, a)
+    })
+  }
+
   /**
    * List of all [[UnaryNode]]s which allow foldable propagation.
    */
   private def canPropagateFoldables(u: UnaryNode): Boolean = u match {
     // Handling `Project` is moved to `propagateFoldables`.
     case _: Filter => true
     case _: SubqueryAlias => true
-    case _: Aggregate => true
+    // Handling `Aggregate` is moved to `propagateFoldables`.

Review comment:
       Ok, will change it.




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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #29816: [WIP][SPARK-32951][SQL] Foldable propagation from Aggregate

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29816:
URL: https://github.com/apache/spark/pull/29816#issuecomment-696211670






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