[SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE #46830

liuzqt · 2024-05-31T22:04:35Z

What changes were proposed in this pull request?

Add dedicated node to represent empty relation (AQE only, i.e., AQEPropagateEmptyRelation.scala).

logical.EmptyRelation
execution.EmptyRelationExec

both are leaf node and store the eliminated logical plan as a field.

In order to display the plan in spark UI, I extended SparkPlanInfo to support mix of logical and physical plan.

Spark UI

String representation

AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
   EmptyRelation [plan_id=260]
   +- Join Inner, (key#3 = a#23)
      :- LogicalQueryStage SerializeFromObject [knownnotnull(assertnotnull(input[0, TestData, true])).key AS key#3, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, TestData, true])).value, true, false, true) AS value#4], ShuffleQueryStage 0
      +- LogicalQueryStage Filter (isnotnull(b#24) AND (b#24 = 1)), ShuffleQueryStage 1
+- == Initial Plan ==
   SortMergeJoin [key#3], [a#23], Inner
   :- Sort [key#3 ASC NULLS FIRST], false, 0
   :  +- Exchange hashpartitioning(key#3, 200), ENSURE_REQUIREMENTS, [plan_id=204]
   :   ...

Why are the changes needed?

Currently we replace with a LocalTableScan in case of empty relation propagation, which lost the information about the original query plan and make it less human readable. The idea is to create a dedicated EmptyRelation node which is a leaf node but wraps the original query plan inside.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

Existing tests

Was this patch authored or co-authored using generative AI tooling?

NO

liuzqt · 2024-06-03T23:02:21Z

@cloud-fan @maryannxue

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlanInfo.scala

cloud-fan · 2024-06-04T23:00:28Z

sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEPropagateEmptyRelation.scala

 object AQEPropagateEmptyRelation extends PropagateEmptyRelationBase {
  override protected def isEmpty(plan: LogicalPlan): Boolean =
-    super.isEmpty(plan) || (!isRootRepartition(plan) && getEstimatedRowCount(plan).contains(0))
+    super.isEmpty(plan) || plan.isInstanceOf[EmptyRelation] ||


do we need to check the type explicitly? It has row count which should be sufficient.

ping @liuzqt , I think this change is not needed as we have https://github.com/apache/spark/pull/46830/files#diff-d72cf9d12043beb25760fb2755ab0dd7fdd881483068c833541a126a1a38b780R32

@cloud-fan we still need to check this explicitly. getEstimatedRowCount only match for specific patterns. Before the change, the reason why it works is because super.isEmpty explicitly match empty LocalRelation.

We can also match EmptyRelation in getEstimatedRowCount alternatively. I don't have preference.

I think it's more reasonable to make getEstimatedRowCount recognize this new EmptyRelation

sql/core/src/main/scala/org/apache/spark/sql/execution/EmptyRelationExec.scala

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlanInfo.scala

cloud-fan · 2024-06-19T08:03:05Z

the failed pyspark test is unrelated, thanks, merging to master!

…onExec ### What changes were proposed in this pull request? Fixed a missing pattern match introduced in #46830 Sorry for the silly mistake... ### Why are the changes needed? ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Existing tests ### Was this patch authored or co-authored using generative AI tooling? NO Closes #47089 from liuzqt/SPARK-48466. Authored-by: Ziqi Liu <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

### What changes were proposed in this pull request? Remove cleanupResource() from`EmptyRelationExec` ### Why are the changes needed? This bug was introduced in #46830 : `cleanupResources` might be executed on the executor where `logical` is null. After revisiting cleanupResources relevant code paths, I think `EmptyRelationExec` doesn't need to anything here. - for driver side cleanup, we have [this code path](https://github.com/apache/spark/blob/0602020eb3b346a8c50ad32eeda4e6dabb70c584/sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala) to cleanup each AQE query stage. - for executor side cleanup, so far we only have SortMergeJoinExec which invoke cleanupResource during its execution, so upon the time when EmptyRelationExec is created, it's guaranteed necessary cleanup has been done. - After all, `EmptyRelationExec` is only a never-execute wrapper for materialized physical query stages, it should not be responsible for any cleanup invocation. So I'm removing `cleanupResources` implementation from `EmptyRelationExec`. ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? New unit test. ### Was this patch authored or co-authored using generative AI tooling? NO Closes #47931 from liuzqt/SPARK-49460. Authored-by: Ziqi Liu <[email protected]> Signed-off-by: yangjie01 <[email protected]>

…kPlanInfo ### What changes were proposed in this pull request? Fix `SparkPlanInfo.fromLogicalPlan` to handle nested empty relation. Before: <img width="294" alt="Screenshot 2025-03-21 at 11 30 56 AM" src="https://github.com/user-attachments/assets/d03f6b88-13ad-4b67-bc4d-18b532e4dea2" /> After: <img width="390" alt="Screenshot 2025-03-20 at 5 51 21 PM" src="https://github.com/user-attachments/assets/0e4f775c-b9cf-4955-af17-5e47fa44e44b" /> ### Why are the changes needed? A followup for #46830, in the original PR I forget to handle nested empty relation. ### Does this PR introduce _any_ user-facing change? Yes, UI change ### How was this patch tested? Verifed in Spark UI ### Was this patch authored or co-authored using generative AI tooling? NO Closes #50350 from liuzqt/SPARK-48466. Authored-by: liuzqt <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…kPlanInfo ### What changes were proposed in this pull request? Fix `SparkPlanInfo.fromLogicalPlan` to handle nested empty relation. Before: <img width="294" alt="Screenshot 2025-03-21 at 11 30 56 AM" src="https://github.com/user-attachments/assets/d03f6b88-13ad-4b67-bc4d-18b532e4dea2" /> After: <img width="390" alt="Screenshot 2025-03-20 at 5 51 21 PM" src="https://github.com/user-attachments/assets/0e4f775c-b9cf-4955-af17-5e47fa44e44b" /> ### Why are the changes needed? A followup for #46830, in the original PR I forget to handle nested empty relation. ### Does this PR introduce _any_ user-facing change? Yes, UI change ### How was this patch tested? Verifed in Spark UI ### Was this patch authored or co-authored using generative AI tooling? NO Closes #50350 from liuzqt/SPARK-48466. Authored-by: liuzqt <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit b8e0246) Signed-off-by: Wenchen Fan <[email protected]>

…kPlanInfo ### What changes were proposed in this pull request? Fix `SparkPlanInfo.fromLogicalPlan` to handle nested empty relation. Before: <img width="294" alt="Screenshot 2025-03-21 at 11 30 56 AM" src="https://github.com/user-attachments/assets/d03f6b88-13ad-4b67-bc4d-18b532e4dea2" /> After: <img width="390" alt="Screenshot 2025-03-20 at 5 51 21 PM" src="https://github.com/user-attachments/assets/0e4f775c-b9cf-4955-af17-5e47fa44e44b" /> ### Why are the changes needed? A followup for apache#46830, in the original PR I forget to handle nested empty relation. ### Does this PR introduce _any_ user-facing change? Yes, UI change ### How was this patch tested? Verifed in Spark UI ### Was this patch authored or co-authored using generative AI tooling? NO Closes apache#50350 from liuzqt/SPARK-48466. Authored-by: liuzqt <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 4661833) Signed-off-by: Wenchen Fan <[email protected]>

dedicated node for EmptyRelation

7f4b874

github-actions bot added the SQL label May 31, 2024

liuzqt added 2 commits May 31, 2024 17:38

fix

192a00a

fix

9e65fa5

HyukjinKwon changed the title ~~[SPARK-48466] Create dedicated node for EmptyRelation in AQE~~ [SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE Jun 3, 2024

fix test

76a9c26

cloud-fan reviewed Jun 4, 2024

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlanInfo.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Jun 4, 2024

View reviewed changes

liuzqt force-pushed the SPARK-48466 branch from 030cda0 to 3162574 Compare June 7, 2024 00:40

refactor: store logical plan in EmptyRelationExec

d4b21ec

liuzqt force-pushed the SPARK-48466 branch from 3162574 to d4b21ec Compare June 7, 2024 18:38

cloud-fan reviewed Jun 11, 2024

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/EmptyRelationExec.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Jun 11, 2024

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/EmptyRelationExec.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Jun 11, 2024

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlanInfo.scala Outdated Show resolved Hide resolved

liuzqt added 3 commits June 11, 2024 23:21

address comment

d435954

minor

85328c3

fix

61aba24

liuzqt requested a review from cloud-fan June 12, 2024 19:59

address comment

d0ca187

cloud-fan closed this in 2fe0692 Jun 19, 2024

liuzqt mentioned this pull request Jun 25, 2024

[SPARK-48466][SQL][FOLLOWUP] Fix missing pattern match in EmptyRelationExec #47089

Closed

tgravescs mentioned this pull request Jun 26, 2024

[AUDIT] [SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE NVIDIA/spark-rapids#11100

Closed

liuzqt mentioned this pull request Aug 29, 2024

[SPARK-49460][SQL] Remove cleanupResource() from EmptyRelationExec #47931

Closed

liuzqt mentioned this pull request Mar 21, 2025

[SPARK-48466][SQL][UI][FOLLOWUP] Handle nested empty relation in SparkPlanInfo #50350

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE #46830

[SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE #46830

Uh oh!

liuzqt commented May 31, 2024 •

edited

Loading

Uh oh!

liuzqt commented Jun 3, 2024

Uh oh!

Uh oh!

cloud-fan Jun 4, 2024

Uh oh!

cloud-fan Jun 11, 2024

Uh oh!

liuzqt Jun 12, 2024 •

edited

Loading

Uh oh!

cloud-fan Jun 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Jun 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE #46830

[SPARK-48466][SQL] Create dedicated node for EmptyRelation in AQE #46830

Uh oh!

Conversation

liuzqt commented May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Spark UI

String representation

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

liuzqt commented Jun 3, 2024

Uh oh!

Uh oh!

cloud-fan Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 11, 2024

Choose a reason for hiding this comment

Uh oh!

liuzqt Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Jun 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

liuzqt commented May 31, 2024 •

edited

Loading

liuzqt Jun 12, 2024 •

edited

Loading