Skip to content

Conversation

@tedyu
Copy link
Contributor

@tedyu tedyu commented Dec 10, 2024

What changes were proposed in this pull request?

As follow-up to #48915, this PR renames isContainsUnsupportedLCA function

Why are the changes needed?

The renamed function is easier to understand.

Does this PR introduce any user-facing change?

No

How was this patch tested?

No change in functionality.
Existing tests suffice.

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

No

@github-actions github-actions bot added the SQL label Dec 10, 2024
@tedyu
Copy link
Contributor Author

tedyu commented Dec 10, 2024

cc @cloud-fan @MaxGekk

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

Could you replace the tag [Minor] by [SPARK-49349][SQL][FOLLOWUP], please.

@tedyu tedyu changed the title [Minor] Rename isContainsUnsupportedLCA function [SPARK-49349][SQL][FOLLOWUP] Rename isContainsUnsupportedLCA function Dec 10, 2024
@tedyu
Copy link
Contributor Author

tedyu commented Dec 10, 2024

@MaxGekk
I have renamed the title of the PR.
Tests passed.

}

private def isContainsUnsupportedLCA(e: Expression, operator: LogicalPlan): Boolean = {
private def containsUnsupportedLCA(e: Expression, operator: LogicalPlan): Boolean = {
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid having such typo fixes ... .this is really something we can easily fix when we happen to touch those codes later...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bq. when we happen to touch those codes later
Agreed

@HyukjinKwon
Copy link
Member

gonna merge this as the PR is open already.

@HyukjinKwon
Copy link
Member

Merged to master.

@tedyu tedyu deleted the cont-unsup branch December 11, 2024 00:35
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