Skip to content

[SPARK-54835][SQL] Avoid unnecessary temp QueryExecution for nested command execution - #53596

Closed
cloud-fan wants to merge 4 commits into
apache:masterfrom
cloud-fan:command
Closed

cloud-fan wants to merge 4 commits into
apache:masterfrom
cloud-fan:command

Conversation

@cloud-fan

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR is a small refactor. In DS v2 CRAS/RTAS command, we run a nested AppendData/OverwriteByExpression command by creating a QueryExecution. This QueryExecution will create another temp QueryExecution to eagerly execute commands. This PR avoids the unnecessary temp QueryExecution by using CommandExecutionMode.SKIP to create QueryExecution.

Why are the changes needed?

Remove useless temp QueryExecution objects.

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?

cursor 2.2.43

@github-actions github-actions Bot added the SQL label Dec 24, 2025
}
}

test("CTAS/RTAS should trigger two query executions") {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test can pass before this PR, I'm adding it just to make sure this refactor doesn't break it and still reports two executions.

@cloud-fan

Copy link
Copy Markdown
Contributor Author

cc @pan3793

Comment thread sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala Outdated
@zhengruifeng

Copy link
Copy Markdown
Contributor

merged to master

dongjoon-hyun pushed a commit that referenced this pull request Feb 20, 2026
…ring listener in test

### What changes were proposed in this pull request?

This is a followup to #53596. In the test "CTAS/RTAS should trigger two query executions", we drain the listener bus before registering the `QueryExecutionListener`, to avoid events from other tests breaking this test with unexpected `executionCount` increments.

### Why are the changes needed?

Without draining the listener bus first, pending events from previous tests could fire after the listener is registered, causing flaky test failures. Other test suites follow this same pattern.

### Does this PR introduce _any_ user-facing change?

No. Test-only change.

### How was this patch tested?

Existing test.

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

Yes.

Made with [Cursor](https://cursor.com)

Closes #54397 from cloud-fan/SPARK-54835-followup.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants