Skip to content
New issue

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

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

Already on GitHub? # to your account

[SPARK-49811][SQL]Rename StringTypeAnyCollation #48265

Closed

Conversation

jovanpavl-db
Copy link
Contributor

What changes were proposed in this pull request?

Rename StringTypeAnyCollation to StringTypeWithCaseAccentSensitivity. Name StringTypeAnyCollation is unfortunate, with adding new type of collations it requires ren

Why are the changes needed?

Name StringTypeAnyCollation is unfortunate, with adding new specifier (for example trim specifier) it requires always renaming it to (something like AllCollationExeptTrimCollation) until new collation is implemented in all functions. It gets even more confusing if multiple collations are not supported for some functions.
Instead of this naming convention should be only specifiers that are supported and avoid using all.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Just renaming all tests passing.

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

No.

@github-actions github-actions bot added the SQL label Sep 26, 2024
@cloud-fan
Copy link
Contributor

BTW, please create JIRA ticker and link it in the PR title.

@stevomitric
Copy link
Contributor

BTW, please create JIRA ticker and link it in the PR title.

Also the [SQL] tag as well.

Copy link
Contributor

@stevomitric stevomitric left a comment

Choose a reason for hiding this comment

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

lgtm

@jovanpavl-db jovanpavl-db changed the title Rename StringTypeAnyCollation [SQL]Rename StringTypeAnyCollation Sep 27, 2024
@jovanpavl-db jovanpavl-db changed the title [SQL]Rename StringTypeAnyCollation [SPARK-49811][SQL]Rename StringTypeAnyCollation Sep 27, 2024
@jovanpavl-db
Copy link
Contributor Author

BTW, please create JIRA ticker and link it in the PR title.

Done.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in d85e7bc Sep 30, 2024
attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
### What changes were proposed in this pull request?
Rename StringTypeAnyCollation to StringTypeWithCaseAccentSensitivity. Name StringTypeAnyCollation is unfortunate, with adding new type of collations it requires ren

### Why are the changes needed?
Name StringTypeAnyCollation is unfortunate, with adding new specifier (for example trim specifier) it requires always renaming it to (something like AllCollationExeptTrimCollation) until new collation is implemented in all functions. It gets even more confusing if multiple collations are not supported for some functions.
Instead of this naming convention should be only specifiers that are supported and avoid using all.

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

### How was this patch tested?
Just renaming all tests passing.

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

Closes apache#48265 from jovanpavl-db/rename-string-type-collations.

Authored-by: Jovan Pavlovic <jovan.pavlovic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
himadripal pushed a commit to himadripal/spark that referenced this pull request Oct 19, 2024
### What changes were proposed in this pull request?
Rename StringTypeAnyCollation to StringTypeWithCaseAccentSensitivity. Name StringTypeAnyCollation is unfortunate, with adding new type of collations it requires ren

### Why are the changes needed?
Name StringTypeAnyCollation is unfortunate, with adding new specifier (for example trim specifier) it requires always renaming it to (something like AllCollationExeptTrimCollation) until new collation is implemented in all functions. It gets even more confusing if multiple collations are not supported for some functions.
Instead of this naming convention should be only specifiers that are supported and avoid using all.

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

### How was this patch tested?
Just renaming all tests passing.

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

Closes apache#48265 from jovanpavl-db/rename-string-type-collations.

Authored-by: Jovan Pavlovic <jovan.pavlovic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants