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

[liuzqt]

What changes were proposed in this pull request?

Remove cleanupResource() fromEmptyRelationExec

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

[liuzqt]

@LuciferYang Thank you for the review! updated

[liuzqt]

I would like to hold on this PR a bit, want to dig more into when cleanupResources should be called and whether we should somehow cleanup this query stage in executor

[liuzqt]

After revisiting cleanupResources relevant code paths, I think EmptyRelationExec doesn't need to anything here.

• for driver side cleanup, we have this code path 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.

[LuciferYang]

[error] /home/runner/work/spark/spark/sql/core/src/main/scala/org/apache/spark/sql/execution/EmptyRelationExec.scala:25:48: Unused import
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=unused-imports, site=org.apache.spark.sql.execution
[error] import org.apache.spark.sql.execution.adaptive.LogicalQueryStage
[error]                                                ^

[LuciferYang]

After revisiting cleanupResources relevant code paths, I think EmptyRelationExec doesn't need to anything here.

• for driver side cleanup, we have this code path 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.

Could you update the title and description of the PR to make them more aligned with the current modifications?

[liuzqt]

@LuciferYang Updated. Could you pls help merge this PR? Thanks!

[LuciferYang]

+1, LGTM

[LuciferYang]

Merged into master. Thanks @liuzqt

[LuciferYang]

@liuzqt I would like to confirm, does this issue really exist in 3.5.x? It seems that EmptyRelationExec does not exist in branch-3.5.

[liuzqt]

@LuciferYang sorry my bad, it doesn't exist in 3.5, so merging into master should be enough. I also updated the Jira ticket. Thank you for the reivew!

[LuciferYang]

@LuciferYang sorry my bad, it doesn't exist in 3.5, so merging into master should be enough. I also updated the Jira ticket. Thank you for the reivew!

Thanks for your confirmation