Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries #49386

Closed
wants to merge 9 commits into from

Conversation

ueshin
Copy link
Member

@ueshin ueshin commented Jan 6, 2025

What changes were proposed in this pull request?

Supports withColumns / withColumnsRenamed in subqueries.

Why are the changes needed?

When the query is used as a subquery by adding col.outer(), withColumns or withColumnsRenamed doesn't work because they need analyzed plans.

Does this PR introduce any user-facing change?

Yes, those APIs are available in subqueries.

How was this patch tested?

Added the related tests.

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

No.

@HyukjinKwon
Copy link
Member

cc @cloud-fan too

@ueshin ueshin requested a review from cloud-fan January 9, 2025 19:18
@@ -1275,29 +1275,15 @@ class Dataset[T] private[sql](
require(colNames.size == cols.size,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check still needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's still good to have to be fail-fast, especially for connect?

@ueshin ueshin changed the title [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries Jan 11, 2025
@ueshin
Copy link
Member Author

ueshin commented Jan 11, 2025

SparkConnectPlanner has an issue. Waiting for #49449.

@ueshin
Copy link
Member Author

ueshin commented Jan 14, 2025

The remaining test failures are not related to this PR.

@ueshin
Copy link
Member Author

ueshin commented Jan 14, 2025

Thanks! merging to master.

@ueshin ueshin closed this in 2273139 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants