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/03/05 07:51:53 UTC
[GitHub] [spark] JkSelf opened a new pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
JkSelf opened a new pull request #31756:
URL: https://github.com/apache/spark/pull/31756
<!--
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'.
-->
### What changes were proposed in this pull request?
We have supported DPP in AQE when the join is Broadcast hash join before applying the AQE rules in [SPARK-34168](https://issues.apache.org/jira/browse/SPARK-34168), which has some limitations. It only apply DPP when the small table side executed firstly and then the big table side can reuse the broadcast exchange in small table side. This PR is to address the above limitations and can apply the DPP when the broadcast exchange can be reused.
<!--
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.
-->
### Why are the changes needed?
Resolve the limitations when both enabling DPP and AQE
<!--
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.
-->
### Does this PR introduce _any_ user-facing change?
No
<!--
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'.
-->
### How was this patch tested?
Adding new ut
<!--
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.
-->
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-821088565
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137466/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838329497
**[Test build #138367 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138367/testReport)** for PR 31756 at commit [`4ccd4b8`](https://github.com/apache/spark/commit/4ccd4b8b1d2604f05a4564303d0b8aa65ea5dfa3).
* 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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-809981767
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41278/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820924080
**[Test build #137466 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137466/testReport)** for PR 31756 at commit [`c7b3735`](https://github.com/apache/spark/commit/c7b3735e12e04b68abd33adbace0653a09a7231a).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838295162
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42899/
--
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] JkSelf commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-795007122
@cloud-fan
This approach mainly contain three steps.
1. Find the reused exchange. If exist, it will apply the DPP filter.
2. In order to reuse the exchange stored in `AdaptiveExecutionContext#stageCache`, we wrap the `AdaptiveSparkPlanExec `plan in `SubqueryAdaptiveBroadcastExec`.
3. `SubqueryAdaptiveBroadcastExec#executeCollect `will reuse the exchange in runtime by calling the `AdaptiveSparkPlanExec#getFinalPhysicalPlan()`
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-809989816
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41278/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833398595
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42724/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820239339
**[Test build #137409 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137409/testReport)** for PR 31756 at commit [`1ed23c5`](https://github.com/apache/spark/commit/1ed23c52b2929682824c44bb47db1bf15cbeb4e4).
--
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] JkSelf commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820386263
@cloud-fan The existing tests in DynamicPartitionPruningSuiteAEOn can cover this changes. We may not need additional tests.
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629426175
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
Review comment:
nit: `SPARK-34637: DPP ...` remove the `test`
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820284722
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41986/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r615904030
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
Review comment:
subquery has different symbols in the tree string format. please try to explain some plans locally and update this comment.
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r627220217
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
This broadcast is in the DPP subquery of the FileScan. It will broadcast the results of the build side and then prune the dataset.
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r615904569
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
there is no other place to reuse this broadcast, right?
--
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] JkSelf commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833362251
@tgravescs Sorry for the delay responses.
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r596855011
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +42,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ plan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(adaptivePlan.executedPlan)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(adaptivePlan.executedPlan)
+ case _ => false
+ }.isDefined
+
+ if(canReuseExchange) {
+ val mode = HashedRelationBroadcastMode(packedKeys)
+ // plan a broadcast exchange of the build side of the join
+ val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
+ exchange.setLogicalLink(adaptivePlan.executedPlan.logicalLink.get)
+ val newAdaptivePlan = AdaptiveSparkPlanExec(
Review comment:
to double-check, the core change is: now we allow the run the broadcast in DPP first, and the corresponding broadcast in other parts of the plan will be reused later. Do I understand correctly?
If the broadcast in other parts of the plan is running (submitted) but not finished, will we run the broadcast twice?
----------------------------------------------------------------
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] JkSelf commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791231318
@cloud-fan Please help me review if you have available time. Thanks for your help.
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833397159
--
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 closed pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31756:
URL: https://github.com/apache/spark/pull/31756
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820391093
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137410/
--
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] tgravescs commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r622256825
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
how is broadcast before the FileScan? what is being broadcast
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833359784
**[Test build #138203 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138203/testReport)** for PR 31756 at commit [`6b07c84`](https://github.com/apache/spark/commit/6b07c847399250def438e0e1b7bbb5b5effa77af).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r613940006
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -310,6 +310,11 @@ case class AdaptiveSparkPlanExec(
rdd
}
+ override def doExecuteBroadcast[T](): broadcast.Broadcast[T] = {
+ val broadcastPlan = getFinalPhysicalPlan()
+ broadcastPlan.doExecuteBroadcast()
Review comment:
nit: `getFinalPhysicalPlan().doExecuteBroadcast()`
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629426795
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
Review comment:
view1 -> v?
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-810049744
**[Test build #136696 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136696/testReport)** for PR 31756 at commit [`ae6fe64`](https://github.com/apache/spark/commit/ae6fe64a325fbce0cade6117f919f2c417f8798a).
* 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] tgravescs commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833511912
made this comment in one of the thread but it got collapse so making it again.
Could you add more description to the PR as to what you are doing and how this solves the problem?
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-810074112
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136696/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-821075848
**[Test build #137466 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137466/testReport)** for PR 31756 at commit [`c7b3735`](https://github.com/apache/spark/commit/c7b3735e12e04b68abd33adbace0653a09a7231a).
* 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] cloud-fan commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820307682
In general LGTM, can we add some tests?
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820585854
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137418/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820392440
**[Test build #137418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137418/testReport)** for PR 31756 at commit [`657c61b`](https://github.com/apache/spark/commit/657c61b7c0e08d085cc505ce0a699fa45fe3e078).
--
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] tgravescs commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r622256825
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
how is broadcast after the FileScan?
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791246183
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40380/
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833545785
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138203/
--
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] github-actions[bot] commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820236797
**[Test build #751342356](https://github.com/JkSelf/spark/actions/runs/751342356)** for PR 31756 at commit [`1ed23c5`](https://github.com/JkSelf/spark/commit/1ed23c52b2929682824c44bb47db1bf15cbeb4e4).
--
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] github-actions[bot] commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820921637
**[Test build #754518485](https://github.com/JkSelf/spark/actions/runs/754518485)** for PR 31756 at commit [`c7b3735`](https://github.com/JkSelf/spark/commit/c7b3735e12e04b68abd33adbace0653a09a7231a).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820948630
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-840542394
thanks, merging 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] AmplabJenkins commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-821088565
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137466/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791235958
**[Test build #135798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135798/testReport)** for PR 31756 at commit [`547ac92`](https://github.com/apache/spark/commit/547ac92075fb1e2a2aa87c91578ffb11e6b733c6).
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820951540
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42041/
--
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] github-actions[bot] commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820245084
**[Test build #751377372](https://github.com/JkSelf/spark/actions/runs/751377372)** for PR 31756 at commit [`3bc4baf`](https://github.com/JkSelf/spark/commit/3bc4baf59335f63f003adf74c87ddb2992e515cc).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838063439
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42890/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-794963801
Can you briefly introduce your approach?
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838222479
**[Test build #138376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138376/testReport)** for PR 31756 at commit [`701f1c3`](https://github.com/apache/spark/commit/701f1c30bbca2bfdffd121cc9c0d6b4a5250f823).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820287557
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41985/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820283698
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r614024613
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -91,10 +91,15 @@ case class AdaptiveSparkPlanExec(
DisableUnnecessaryBucketedScan
) ++ context.session.sessionState.queryStagePrepRules
+ @transient private val initialPlan = context.session.withActive {
+ applyPhysicalRules(
+ inputPlan, queryStagePreparationRules, Some((planChangeLogger, "AQE Preparations")))
+ }
+
// A list of physical optimizer rules to be applied to a new stage before its execution. These
// optimizations should be stage-independent.
@transient private val queryStageOptimizerRules: Seq[Rule[SparkPlan]] = Seq(
- PlanAdaptiveDynamicPruningFilters(context.stageCache),
+ PlanAdaptiveDynamicPruningFilters(initialPlan),
Review comment:
We need use the `initialPlan `not the `inputPlan`, because the `inputPlan `is not applied the `queryStagePreparationRules(EnsureRequirements)`.
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820284404
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41985/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820287592
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41985/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838063341
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820287592
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41985/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820246320
**[Test build #137410 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137410/testReport)** for PR 31756 at commit [`3bc4baf`](https://github.com/apache/spark/commit/3bc4baf59335f63f003adf74c87ddb2992e515cc).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820240529
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137409/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629430388
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
Can you make it more clear that this is the subquery in the file scan node, not the child of 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 commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833398595
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42724/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820410517
@JkSelf do we have a test that covers the case that the broadcast query stage is created at the DPP side 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] SparkQA commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833359784
**[Test build #138203 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138203/testReport)** for PR 31756 at commit [`6b07c84`](https://github.com/apache/spark/commit/6b07c847399250def438e0e1b7bbb5b5effa77af).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838597502
**[Test build #138376 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138376/testReport)** for PR 31756 at commit [`701f1c3`](https://github.com/apache/spark/commit/701f1c30bbca2bfdffd121cc9c0d6b4a5250f823).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class PlanAdaptiveDynamicPruningFilters(`
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629425441
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -44,12 +42,23 @@ case class PlanAdaptiveDynamicPruningFilters(
val mode = HashedRelationBroadcastMode(packedKeys)
// plan a broadcast exchange of the build side of the join
val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ rootPlan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(exchange)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(exchange)
+ case _ => false
+ }.isDefined
+
+ if (canReuseExchange) {
+ exchange.setLogicalLink(adaptivePlan.executedPlan.logicalLink.get)
+ val newAdaptivePlan = AdaptiveSparkPlanExec(
+ exchange, adaptivePlan.context, adaptivePlan.preprocessingRules, true)
Review comment:
ditto: `adaptivePlan.copy(inputPlan = exchange)`
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820284722
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41986/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-809955073
**[Test build #136696 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136696/testReport)** for PR 31756 at commit [`ae6fe64`](https://github.com/apache/spark/commit/ae6fe64a325fbce0cade6117f919f2c417f8798a).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791246183
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791317467
**[Test build #135798 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135798/testReport)** for PR 31756 at commit [`547ac92`](https://github.com/apache/spark/commit/547ac92075fb1e2a2aa87c91578ffb11e6b733c6).
* 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] AmplabJenkins removed a comment on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838295162
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42899/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629983300
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -91,10 +91,15 @@ case class AdaptiveSparkPlanExec(
DisableUnnecessaryBucketedScan
) ++ context.session.sessionState.queryStagePrepRules
+ @transient private val initialPlan = context.session.withActive {
+ applyPhysicalRules(
+ inputPlan, queryStagePreparationRules, Some((planChangeLogger, "AQE Preparations")))
+ }
+
// A list of physical optimizer rules to be applied to a new stage before its execution. These
// optimizations should be stage-independent.
@transient private val queryStageOptimizerRules: Seq[Rule[SparkPlan]] = Seq(
- PlanAdaptiveDynamicPruningFilters(context.stageCache),
+ PlanAdaptiveDynamicPruningFilters(initialPlan),
Review comment:
I think it's better to pass `this` as the root plan. `AdaptiveSparkPlanExec` keeps changing when more and more query stages are completed. So it's better that `PlanAdaptiveDynamicPruningFilters` always look at the latest plan.
--
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] github-actions[bot] commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820379780
**[Test build #751980907](https://github.com/JkSelf/spark/actions/runs/751980907)** for PR 31756 at commit [`657c61b`](https://github.com/JkSelf/spark/commit/657c61b7c0e08d085cc505ce0a699fa45fe3e078).
--
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] tgravescs commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-830080444
@JkSelf will you have time to look at the questions and comments?
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820421613
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838665138
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138376/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-837993646
**[Test build #138367 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138367/testReport)** for PR 31756 at commit [`4ccd4b8`](https://github.com/apache/spark/commit/4ccd4b8b1d2604f05a4564303d0b8aa65ea5dfa3).
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r603806230
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +42,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ plan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(adaptivePlan.executedPlan)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(adaptivePlan.executedPlan)
+ case _ => false
+ }.isDefined
+
+ if(canReuseExchange) {
+ val mode = HashedRelationBroadcastMode(packedKeys)
+ // plan a broadcast exchange of the build side of the join
+ val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
+ exchange.setLogicalLink(adaptivePlan.executedPlan.logicalLink.get)
+ val newAdaptivePlan = AdaptiveSparkPlanExec(
Review comment:
Yes. The broadcast twice issue will occur. And I add the wait mechanism to avoid this issue.
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r614575144
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
Review comment:
@cloud-fan Add new test to test the DPP side broadcast query stage is created firstly.
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820246320
**[Test build #137410 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137410/testReport)** for PR 31756 at commit [`3bc4baf`](https://github.com/apache/spark/commit/3bc4baf59335f63f003adf74c87ddb2992e515cc).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833545785
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138203/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820924080
**[Test build #137466 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137466/testReport)** for PR 31756 at commit [`c7b3735`](https://github.com/apache/spark/commit/c7b3735e12e04b68abd33adbace0653a09a7231a).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838375570
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138367/
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629918830
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -44,12 +42,23 @@ case class PlanAdaptiveDynamicPruningFilters(
val mode = HashedRelationBroadcastMode(packedKeys)
// plan a broadcast exchange of the build side of the join
val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ rootPlan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(exchange)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(exchange)
+ case _ => false
+ }.isDefined
+
+ if (canReuseExchange) {
+ exchange.setLogicalLink(adaptivePlan.executedPlan.logicalLink.get)
+ val newAdaptivePlan = AdaptiveSparkPlanExec(
+ exchange, adaptivePlan.context, adaptivePlan.preprocessingRules, true)
Review comment:
updated.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
Review comment:
updated.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
Review comment:
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] tgravescs commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r622256598
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +42,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ plan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(adaptivePlan.executedPlan)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(adaptivePlan.executedPlan)
+ case _ => false
+ }.isDefined
+
+ if(canReuseExchange) {
+ val mode = HashedRelationBroadcastMode(packedKeys)
+ // plan a broadcast exchange of the build side of the join
+ val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
+ exchange.setLogicalLink(adaptivePlan.executedPlan.logicalLink.get)
+ val newAdaptivePlan = AdaptiveSparkPlanExec(
Review comment:
I'm trying to understand this change. Could you add more description to the PR as to what you are doing and how this solves the problem?
It seems now we create a AdaptiveSparkPlanExec inside another AdaptiveSparkPlanExec where that one just does the broadcast. I guess that one has the DPP already so then the existing logic checking stagecache will reuse the DPP one after that?
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838063439
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42890/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r613941099
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +40,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ originalPlan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(adaptivePlan.executedPlan)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(adaptivePlan.executedPlan)
+ case _ => false
+ }.isDefined
+
+ if(canReuseExchange) {
Review comment:
nit: `if (canReuseExchange)`
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820239339
**[Test build #137409 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137409/testReport)** for PR 31756 at commit [`1ed23c5`](https://github.com/apache/spark/commit/1ed23c52b2929682824c44bb47db1bf15cbeb4e4).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820391093
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-809989816
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41278/
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838222479
**[Test build #138376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138376/testReport)** for PR 31756 at commit [`701f1c3`](https://github.com/apache/spark/commit/701f1c30bbca2bfdffd121cc9c0d6b4a5250f823).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629432669
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // Dynamicpruning Subquery
Review comment:
Did try to explain some queries locally? If you did you should see how subqueries are displayed. For example, `select 1, (select 2)`:
```
Project [1 AS 1#7, scalar-subquery#6 [] AS scalarsubquery()#9]
: +- Project [2 AS 2#8]
: +- OneRowRelation
+- OneRowRelation
```
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-837993646
**[Test build #138367 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138367/testReport)** for PR 31756 at commit [`4ccd4b8`](https://github.com/apache/spark/commit/4ccd4b8b1d2604f05a4564303d0b8aa65ea5dfa3).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r613940338
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -17,19 +17,18 @@
package org.apache.spark.sql.execution.adaptive
-import scala.collection.concurrent.TrieMap
-
import org.apache.spark.sql.catalyst.expressions.{BindReferences, DynamicPruningExpression, Literal}
+import org.apache.spark.sql.catalyst.optimizer.{BuildLeft, BuildRight}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.execution._
import org.apache.spark.sql.execution.exchange.BroadcastExchangeExec
-import org.apache.spark.sql.execution.joins.{HashedRelationBroadcastMode, HashJoin}
+import org.apache.spark.sql.execution.joins.{BroadcastHashJoinExec, HashedRelationBroadcastMode, HashJoin}
/**
* A rule to insert dynamic pruning predicates in order to reuse the results of broadcast.
*/
case class PlanAdaptiveDynamicPruningFilters(
- stageCache: TrieMap[SparkPlan, QueryStageExec]) extends Rule[SparkPlan] {
+ originalPlan: SparkPlan) extends Rule[SparkPlan] {
Review comment:
`rootPlan`
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r591142146
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1409,4 +1409,17 @@ class DynamicPartitionPruningSuiteAEOff extends DynamicPartitionPruningSuiteBase
with DisableAdaptiveExecutionSuite
class DynamicPartitionPruningSuiteAEOn extends DynamicPartitionPruningSuiteBase
- with EnableAdaptiveExecutionSuite
+ with EnableAdaptiveExecutionSuite {
+ test("simple inner join triggers DPP with mock-up tables test") {
Review comment:
Only for debug. I will remove this test later.
----------------------------------------------------------------
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r614025183
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -310,6 +310,11 @@ case class AdaptiveSparkPlanExec(
rdd
}
+ override def doExecuteBroadcast[T](): broadcast.Broadcast[T] = {
+ val broadcastPlan = getFinalPhysicalPlan()
+ broadcastPlan.doExecuteBroadcast()
Review comment:
Updated.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -17,19 +17,18 @@
package org.apache.spark.sql.execution.adaptive
-import scala.collection.concurrent.TrieMap
-
import org.apache.spark.sql.catalyst.expressions.{BindReferences, DynamicPruningExpression, Literal}
+import org.apache.spark.sql.catalyst.optimizer.{BuildLeft, BuildRight}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.execution._
import org.apache.spark.sql.execution.exchange.BroadcastExchangeExec
-import org.apache.spark.sql.execution.joins.{HashedRelationBroadcastMode, HashJoin}
+import org.apache.spark.sql.execution.joins.{BroadcastHashJoinExec, HashedRelationBroadcastMode, HashJoin}
/**
* A rule to insert dynamic pruning predicates in order to reuse the results of broadcast.
*/
case class PlanAdaptiveDynamicPruningFilters(
- stageCache: TrieMap[SparkPlan, QueryStageExec]) extends Rule[SparkPlan] {
+ originalPlan: SparkPlan) extends Rule[SparkPlan] {
Review comment:
Updated.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +40,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ originalPlan.find {
+ case BroadcastHashJoinExec(_, _, _, BuildLeft, _, left, _, _) =>
+ left.sameResult(adaptivePlan.executedPlan)
+ case BroadcastHashJoinExec(_, _, _, BuildRight, _, _, right, _) =>
+ right.sameResult(adaptivePlan.executedPlan)
+ case _ => false
+ }.isDefined
+
+ if(canReuseExchange) {
Review comment:
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] SparkQA commented on pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820240501
**[Test build #137409 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137409/testReport)** for PR 31756 at commit [`1ed23c5`](https://github.com/apache/spark/commit/1ed23c52b2929682824c44bb47db1bf15cbeb4e4).
* This patch **fails Scala style 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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791317748
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135798/
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r596850988
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +42,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
HashJoin.rewriteKeyExpr(buildKeys), adaptivePlan.executedPlan.output)
- val mode = HashedRelationBroadcastMode(packedKeys)
- // plan a broadcast exchange of the build side of the join
- val exchange = BroadcastExchangeExec(mode, adaptivePlan.executedPlan)
- val existingStage = stageCache.get(exchange.canonicalized)
- if (existingStage.nonEmpty && conf.exchangeReuseEnabled) {
- val name = s"dynamicpruning#${exprId.id}"
- val reuseQueryStage = existingStage.get.newReuseInstance(0, exchange.output)
- val broadcastValues =
- SubqueryBroadcastExec(name, index, buildKeys, reuseQueryStage)
+
+ val canReuseExchange = conf.exchangeReuseEnabled && buildKeys.nonEmpty &&
+ plan.find {
Review comment:
`PlanAdaptiveDynamicPruningFilters` is a stage optimization rule and the input `plan` is only a small piece of the plan tree (for one stage). I think we should put the entire plan as a parameter of this rule, when creating this rule in `AdaptiveSparkPlanExec`
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-791235958
**[Test build #135798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135798/testReport)** for PR 31756 at commit [`547ac92`](https://github.com/apache/spark/commit/547ac92075fb1e2a2aa87c91578ffb11e6b733c6).
----------------------------------------------------------------
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820951540
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42041/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820240529
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137409/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820431169
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41994/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820585854
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137418/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-809955073
**[Test build #136696 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136696/testReport)** for PR 31756 at commit [`ae6fe64`](https://github.com/apache/spark/commit/ae6fe64a325fbce0cade6117f919f2c417f8798a).
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838375570
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138367/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820392440
**[Test build #137418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137418/testReport)** for PR 31756 at commit [`657c61b`](https://github.com/apache/spark/commit/657c61b7c0e08d085cc505ce0a699fa45fe3e078).
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-810074112
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136696/
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820383820
**[Test build #137410 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137410/testReport)** for PR 31756 at commit [`3bc4baf`](https://github.com/apache/spark/commit/3bc4baf59335f63f003adf74c87ddb2992e515cc).
* 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] JkSelf commented on pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-834046989
@tgravescs
This PR is mainly to solve the limitations of [PR#31258](https://github.com/apache/spark/pull/31258). When DPP + AQE is supported in [PR#31258](https://github.com/apache/spark/pull/31258), only the broadcast exchange on the build side can be executed first. Then the probe side can reuse the exchange of the build side in the DPP subquery, otherwise DPP will not be supported in AQE.
This approach mainly contain two steps.
1. In `PlanAdaptiveDynamicPruningFilters` rule, judge whether the broadcast exchange can be reused, if so, it will insert the DPP subquery filter on the probe side.
2. Create a `AdaptiveSparkPlanExec` with the broadcast exchange and then we can reuse the existing reuse logic to reuse the broadcast exchange in `AdaptiveSparkPlanExec` plan。
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838665138
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138376/
--
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r629918664
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // Dynamicpruning Subquery
Review comment:
Yes. In this case, it looks like the following:
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-833522339
**[Test build #138203 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138203/testReport)** for PR 31756 at commit [`6b07c84`](https://github.com/apache/spark/commit/6b07c847399250def438e0e1b7bbb5b5effa77af).
* 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] cloud-fan commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r613941517
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/PlanAdaptiveDynamicPruningFilters.scala
##########
@@ -41,15 +40,26 @@ case class PlanAdaptiveDynamicPruningFilters(
adaptivePlan: AdaptiveSparkPlanExec), exprId, _)) =>
val packedKeys = BindReferences.bindReferences(
Review comment:
we can move this into `if (canReuseExchange)`
--
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 #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-838285766
--
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] maryannxue commented on a change in pull request #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r590799126
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1409,4 +1409,17 @@ class DynamicPartitionPruningSuiteAEOff extends DynamicPartitionPruningSuiteBase
with DisableAdaptiveExecutionSuite
class DynamicPartitionPruningSuiteAEOn extends DynamicPartitionPruningSuiteBase
- with EnableAdaptiveExecutionSuite
+ with EnableAdaptiveExecutionSuite {
+ test("simple inner join triggers DPP with mock-up tables test") {
Review comment:
What does this test?
----------------------------------------------------------------
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] JkSelf commented on a change in pull request #31756: [SPARK-34637] [SQL] Support DPP + AQE when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
JkSelf commented on a change in pull request #31756:
URL: https://github.com/apache/spark/pull/31756#discussion_r627205248
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/DynamicPartitionPruningSuite.scala
##########
@@ -1463,6 +1474,37 @@ abstract class DynamicPartitionPruningSuiteBase
}
}
}
+
+ test("SPARK-34637: test DPP side broadcast query stage is created firstly") {
+ withSQLConf(SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "true") {
+ val df = sql(
+ """ WITH view1 as (
+ | SELECT f.store_id FROM fact_stats f WHERE f.units_sold = 70 group by f.store_id
+ | )
+ |
+ | SELECT * FROM view1 v1 join view1 v2 WHERE v1.store_id = v2.store_id
+ """.stripMargin)
+
+ // A possible resulting query plan:
+ // BroadcastHashJoin
+ // +- HashAggregate
+ // +- ShuffleQueryStage
+ // +- Exchange
+ // +- HashAggregate
+ // +- Filter
+ // +- FileScan
+ // +- SubqueryBroadcast
+ // +- AdaptiveSparkPlan
+ // +- BroadcastQueryStage
Review comment:
This broadcast only be reused in the build side.
--
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 #31756: [SPARK-34637] [SQL] [WIP] Support DPP when the broadcast exchange can be reused
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31756:
URL: https://github.com/apache/spark/pull/31756#issuecomment-820584372
**[Test build #137418 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137418/testReport)** for PR 31756 at commit [`657c61b`](https://github.com/apache/spark/commit/657c61b7c0e08d085cc505ce0a699fa45fe3e078).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class PlanAdaptiveDynamicPruningFilters(rootPlan: SparkPlan) extends Rule[SparkPlan] `
--
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