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/05/04 06:07:24 UTC
[GitHub] [spark] c21 opened a new pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
c21 opened a new pull request #32430:
URL: https://github.com/apache/spark/pull/32430
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error message, please read the guideline first:
https://spark.apache.org/error-message-guidelines.html
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
`EXPLAIN CODEGEN <query>` (and Dataset.explain("codegen")) prints out the generated code for each stage of plan. The current implementation is to match `WholeStageCodegenExec` operator in query plan and prints out generated code (https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala#L111-L118 ). This does not work with AQE as we wrap the whole query plan inside `AdaptiveSparkPlanExec` and do not run whole stage code-gen physical plan rule eagerly (`CollapseCodegenStages`). This introduces unexpected behavior change for EXPLAIN query (and Dataset.explain), as we enable AQE by default now.
The change is to explain code-gen for the original input plan of AQE.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Make `EXPLAIN CODEGEN` work same as before.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No (when comparing with latest Spark release 3.1.1).
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Added unit test in `ExplainSuite.scala`.
--
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 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627825684
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -565,6 +565,33 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
|""".stripMargin
)
}
+
+ test("SPARK-35133: explain codegen should work with AQE") {
+ withSQLConf(SQLConf.WHOLESTAGE_CODEGEN_ENABLED.key -> "true") {
Review comment:
Hi, @c21 .
Although SPARK-33679 enabled AQE by default for Apache Spark 3.2, we don't know what happens during QA and Apache Spark 3.2.0 RC period. In addition, we had better an explicit configuration in the test case. Could you add `spark.sql.adaptive.enabled=true` additionally?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832412488
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138148/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834100665
**[Test build #138227 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138227/testReport)** for PR 32430 at commit [`fae6c5d`](https://github.com/apache/spark/commit/fae6c5d60a87eec3be5f1d5583f3b97f92696e07).
* 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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627209285
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
@cloud-fan - updated, I agree, will check as well, 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] AmplabJenkins commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832363554
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42669/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832327804
**[Test build #138148 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138148/testReport)** for PR 32430 at commit [`2b7b6da`](https://github.com/apache/spark/commit/2b7b6daf98dfbebd1cc9236cc28214bed6620656).
--
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 a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r626996573
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
+ val inputExecutedPlan = QueryExecution.prepareForExecution(
+ QueryExecution.preparations(sparkSession, None), p.inputPlan)
Review comment:
Could we use `SparkSession.getActiveSession` to avoid the unnecessary changes?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833398591
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42722/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834025815
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42749/
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627826556
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -565,6 +565,33 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
|""".stripMargin
)
}
+
+ test("SPARK-35133: explain codegen should work with AQE") {
+ withSQLConf(SQLConf.WHOLESTAGE_CODEGEN_ENABLED.key -> "true") {
Review comment:
@dongjoon-hyun - sorry if it's not clear, this test is under `ExplainSuiteAE` which extends `EnableAdaptiveExecutionSuite` to enforce AQE enabled already.
--
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 a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r625724495
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -197,7 +198,14 @@ class QueryExecution(
queryExecution.toString(maxFields, append)
case CodegenMode =>
try {
- org.apache.spark.sql.execution.debug.writeCodegen(append, queryExecution.executedPlan)
+ queryExecution.executedPlan match {
Review comment:
How about handling the AQE node in the `writeCodegen` side instead? That way, `debugCodegen` can support it, too?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834020134
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833349418
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832541072
**[Test build #138167 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138167/testReport)** for PR 32430 at commit [`788e714`](https://github.com/apache/spark/commit/788e7144df35fb040144768dadf644dbefe0e241).
* 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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627828452
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/debug/DebuggingSuite.scala
##########
@@ -24,14 +24,13 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.execution.{CodegenSupport, LeafExecNode, WholeStageCodegenExec}
-import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite
+import org.apache.spark.sql.execution.adaptive.{DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
import org.apache.spark.sql.functions._
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.test.SQLTestData.TestData
import org.apache.spark.sql.types.StructType
-// Disable AQE because the WholeStageCodegenExec is added when running QueryStageExec
Review comment:
oh do you mean move the comment to line 100?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833317602
**[Test build #138196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138196/testReport)** for PR 32430 at commit [`f6a5b85`](https://github.com/apache/spark/commit/f6a5b85e647c47985b27f2c62201eb809aff8f63).
--
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 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627825684
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -565,6 +565,33 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
|""".stripMargin
)
}
+
+ test("SPARK-35133: explain codegen should work with AQE") {
+ withSQLConf(SQLConf.WHOLESTAGE_CODEGEN_ENABLED.key -> "true") {
Review comment:
Hi, @c21 .
Although SPARK-33679 enabled AQE by default for Apache Spark 3.2, we don't know what happens during QA and Apache Spark 3.2.0 RC period. In addition, we had better an explicit configuration in the test case. Could you add `spark.sql.adaptive.enabled=true` explicitly?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833476082
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138196/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832562149
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138167/
--
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 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627826994
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/debug/DebuggingSuite.scala
##########
@@ -24,14 +24,13 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.execution.{CodegenSupport, LeafExecNode, WholeStageCodegenExec}
-import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite
+import org.apache.spark.sql.execution.adaptive.{DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
import org.apache.spark.sql.functions._
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.test.SQLTestData.TestData
import org.apache.spark.sql.types.StructType
-// Disable AQE because the WholeStageCodegenExec is added when running QueryStageExec
Review comment:
Instead of removal, shall we recover this for `DebuggingSuite` at line 100?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627174159
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
We need a bit more thinking about how to do EXPLAIN with AQE.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #32430:
URL: https://github.com/apache/spark/pull/32430
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627871552
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/debug/DebuggingSuite.scala
##########
@@ -24,14 +24,13 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.execution.{CodegenSupport, LeafExecNode, WholeStageCodegenExec}
-import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite
+import org.apache.spark.sql.execution.adaptive.{DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
import org.apache.spark.sql.functions._
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.test.SQLTestData.TestData
import org.apache.spark.sql.types.StructType
-// Disable AQE because the WholeStageCodegenExec is added when running QueryStageExec
Review comment:
@dongjoon-hyun - updated to move the comment to line 100, let me know if my understanding is not correct, 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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627186678
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
I am fine with changing the code to be `findSubtrees(aqePlan.executedPlan)`, so the explain code-gen behavior is same as explain, and at least it fixes the case for data frame being executed already (is final plan).
I can just add the change for `SQL EXPLAIN` in our internal fork and discuss in the future. WDYT @cloud-fan ?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833398591
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42722/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833359078
**[Test build #138201 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138201/testReport)** for PR 32430 at commit [`4683f0e`](https://github.com/apache/spark/commit/4683f0e73ef12871e88438c08440857c0cdb52bc).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832327804
**[Test build #138148 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138148/testReport)** for PR 32430 at commit [`2b7b6da`](https://github.com/apache/spark/commit/2b7b6daf98dfbebd1cc9236cc28214bed6620656).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832435776
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42688/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627037126
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
This doesn't match the actual plan. Why can't use `AdaptiveSparkPlanExec.executedPlan`?
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627827918
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/debug/DebuggingSuite.scala
##########
@@ -24,14 +24,13 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.execution.{CodegenSupport, LeafExecNode, WholeStageCodegenExec}
-import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite
+import org.apache.spark.sql.execution.adaptive.{DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
import org.apache.spark.sql.functions._
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.test.SQLTestData.TestData
import org.apache.spark.sql.types.StructType
-// Disable AQE because the WholeStageCodegenExec is added when running QueryStageExec
Review comment:
@dongjoon-hyun - sorry can you elaborate more by recovering? Here I kept the AQE-runnable unit tests under `DebuggingSuiteBase`, and have a non-AQE `DebuggingSuite` and AQE `DebuggingSuiteAE`.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627179291
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
After some plan changes (such as join elimination) in AQE, I don't think SQL EXPLAIN is useful as you can never see the final plan. How do you expect SQL EXPLAIN to work for AQE?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832433561
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42688/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834003794
**[Test build #138227 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138227/testReport)** for PR 32430 at commit [`fae6c5d`](https://github.com/apache/spark/commit/fae6c5d60a87eec3be5f1d5583f3b97f92696e07).
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r626246014
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -197,7 +198,14 @@ class QueryExecution(
queryExecution.toString(maxFields, append)
case CodegenMode =>
try {
- org.apache.spark.sql.execution.debug.writeCodegen(append, queryExecution.executedPlan)
+ queryExecution.executedPlan match {
Review comment:
@maropu - updated, 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] AmplabJenkins commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832412488
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138148/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832562149
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138167/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833358050
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42717/
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627071113
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
Yes, this doesn't match the actual plan for `df.explain("codegen")` if `df` is executed already. The problem is the final plan `AdaptiveSparkPlanExec.executedPlan` has `ShuffleQueryStageExec` to wrap the whole sub-plan under that shuffle.
Example:
```
spark.range(5).select(col("id").as("key"), col("id").as("value")).groupBy('key).agg(max('value))
```
```
AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
*(2) HashAggregate(keys=[key#2L], functions=[max(value#3L)], output=[key#2L, max(value)#9L])
+- CustomShuffleReader coalesced
+- ShuffleQueryStage 0
+- Exchange hashpartitioning(key#2L, 5), ENSURE_REQUIREMENTS, [id=#28]
+- *(1) HashAggregate(keys=[key#2L], functions=[partial_max(value#3L)], output=[key#2L, max#13L])
+- *(1) Project [id#0L AS key#2L, id#0L AS value#3L]
+- *(1) Range (0, 5, step=1, splits=2)
```
The partial aggregate `HashAggregate` is wrapped inside `ShuffleQueryStage`, so cannot be pattern matched to do the explain. One way to workaround is to add pattern matching for `ShuffleQueryStageExec` as well. But anyway we need to re-run the preparation physical plan rules if `AdaptiveSparkPlan.isFinalPlan=false`.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627036985
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
This doesn't match the actual plan. Why can't use `AdaptiveSparkPlanExec.executedPlan`?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832409814
**[Test build #138167 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138167/testReport)** for PR 32430 at commit [`788e714`](https://github.com/apache/spark/commit/788e7144df35fb040144768dadf644dbefe0e241).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832363554
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42669/
--
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 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627827978
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -565,6 +565,33 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
|""".stripMargin
)
}
+
+ test("SPARK-35133: explain codegen should work with AQE") {
+ withSQLConf(SQLConf.WHOLESTAGE_CODEGEN_ENABLED.key -> "true") {
Review comment:
Oh, got it. Then, it's perfect.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832411667
**[Test build #138148 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138148/testReport)** for PR 32430 at commit [`2b7b6da`](https://github.com/apache/spark/commit/2b7b6daf98dfbebd1cc9236cc28214bed6620656).
* 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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627183085
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
Because AQE is not changing every query. As an example I gave in unit test case in `ExplainSuite.scala`, for aggregation query, AQE should not affect the behavior for the generated code for aggregation.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627037126
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
This doesn't match the actual plan. Why can't use `AdaptiveSparkPlanExec.executedPlan`?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834119536
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138227/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832456863
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42688/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832353412
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42669/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832351468
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42669/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832409814
**[Test build #138167 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138167/testReport)** for PR 32430 at commit [`788e714`](https://github.com/apache/spark/commit/788e7144df35fb040144768dadf644dbefe0e241).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833545794
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138201/
--
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] c21 commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-831741059
cc @cloud-fan if you have time to take a look, 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] AmplabJenkins commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834119536
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138227/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833514287
**[Test build #138201 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138201/testReport)** for PR 32430 at commit [`4683f0e`](https://github.com/apache/spark/commit/4683f0e73ef12871e88438c08440857c0cdb52bc).
* 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] AmplabJenkins commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833476082
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138196/
--
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] c21 commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834054871
Thank you all for review!
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833358050
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42717/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834003794
**[Test build #138227 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138227/testReport)** for PR 32430 at commit [`fae6c5d`](https://github.com/apache/spark/commit/fae6c5d60a87eec3be5f1d5583f3b97f92696e07).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833393001
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627162402
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
+ val inputExecutedPlan = QueryExecution.prepareForExecution(
+ QueryExecution.preparations(sparkSession, None), p.inputPlan)
Review comment:
@maropu - good call, it's much cleaner. Updated.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627199105
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
SGTM. We can also look at how other systems do EXPLAIN with query plan that can change at runtime.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834025815
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42749/
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833317602
**[Test build #138196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138196/testReport)** for PR 32430 at commit [`f6a5b85`](https://github.com/apache/spark/commit/f6a5b85e647c47985b27f2c62201eb809aff8f63).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-832456863
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42688/
--
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 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627903213
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/debug/DebuggingSuite.scala
##########
@@ -24,14 +24,13 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.execution.{CodegenSupport, LeafExecNode, WholeStageCodegenExec}
-import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecutionSuite
+import org.apache.spark.sql.execution.adaptive.{DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
import org.apache.spark.sql.functions._
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.test.SQLTestData.TestData
import org.apache.spark.sql.types.StructType
-// Disable AQE because the WholeStageCodegenExec is added when running QueryStageExec
Review comment:
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] cloud-fan commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627173903
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
Ah, now I see what you mean. For `isFinalPlan=false`, we may have query stages in the query plan that do not run `queryStageOptimizerRules` yet, and have no `WholeStageCodegenExec`.
I think this is at least consistent with normal EXPLAIN. If the `df` is not run yet, you won't see the leading `*` in plan nodes that indicate whole-stage codegen.
One idea is to trigger AQE during EXPLAIN, so that we can display the final plan. But this won't work for SQL EXPLAIN.
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627162660
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
@cloud-fan - updated.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627115988
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -105,13 +106,20 @@ package object debug {
* @param plan the query plan for codegen
* @return Sequence of WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenStringSeq(plan: SparkPlan): Seq[(String, String, ByteCodeStats)] = {
+ def codegenStringSeq(
+ plan: SparkPlan,
+ sparkSession: SparkSession): Seq[(String, String, ByteCodeStats)] = {
val codegenSubtrees = new collection.mutable.HashSet[WholeStageCodegenExec]()
def findSubtrees(plan: SparkPlan): Unit = {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from original input plan of AQE.
Review comment:
Yea we can strip `QueryStageExec` as well. Can we use `executedPlan` even if `isFinalPlan=false`?
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833545794
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138201/
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627175397
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
@cloud-fan - I feel `SQL EXPLAIN` is usable, at least for me and several developers in the team. It's kind of annoying that we need to disable AQE every time for this.
--
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] c21 commented on a change in pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
c21 commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627178114
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -112,6 +113,19 @@ package object debug {
plan foreach {
case s: WholeStageCodegenExec =>
codegenSubtrees += s
+ case p: AdaptiveSparkPlanExec =>
+ // Find subtrees from current executed plan of AQE.
+ val executedPlan = p.executedPlan
+ if (executedPlan.find(_.isInstanceOf[WholeStageCodegenExec]).isEmpty) {
Review comment:
> One idea is to trigger AQE during EXPLAIN, so that we can display the final plan.
I think users and developers are expecting `EXPLAIN` to be a purely metadata operator, so it might be surprising or inefficient to execute actual query during `EXPLAIN`.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833359078
**[Test build #138201 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138201/testReport)** for PR 32430 at commit [`4683f0e`](https://github.com/apache/spark/commit/4683f0e73ef12871e88438c08440857c0cdb52bc).
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32430:
URL: https://github.com/apache/spark/pull/32430#discussion_r627036460
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
##########
@@ -73,14 +74,14 @@ package object debug {
* @param plan the query plan for codegen
* @return single String containing all WholeStageCodegen subtrees and corresponding codegen
*/
- def codegenString(plan: SparkPlan): String = {
+ def codegenString(plan: SparkPlan, sparkSession: SparkSession): String = {
Review comment:
shall we simply strip these AQE wrapper nodes at the beginning using `AdaptiveSparkPlanHelper.stripAQEPlan`?
--
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 commented on pull request #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-834041404
It seems that there is some delay at GitHub Action. I checked that it's already passed.
<img width="829" alt="Screen Shot 2021-05-06 at 8 42 37 PM" src="https://user-images.githubusercontent.com/9700541/117394778-9eb16a80-aeab-11eb-8e75-e5aee9c93ba7.png">
Thank you, @c21 and all. Merged to master.
--
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 #32430: [SPARK-35133][SQL] Explain codegen works with AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32430:
URL: https://github.com/apache/spark/pull/32430#issuecomment-833474878
**[Test build #138196 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138196/testReport)** for PR 32430 at commit [`f6a5b85`](https://github.com/apache/spark/commit/f6a5b85e647c47985b27f2c62201eb809aff8f63).
* 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