Skip to content

[SPARK-48263] Collate function support for non UTF8_BINARY strings#46574

Closed
nebojsa-db wants to merge 3 commits intoapache:masterfrom
nebojsa-db:SPARK-48263
Closed

[SPARK-48263] Collate function support for non UTF8_BINARY strings#46574
nebojsa-db wants to merge 3 commits intoapache:masterfrom
nebojsa-db:SPARK-48263

Conversation

@nebojsa-db
Copy link
Contributor

@nebojsa-db nebojsa-db commented May 14, 2024

What changes were proposed in this pull request?

collate("xx", "") does not work when there is a config for default collation set which configures non UTF8_BINARY collation as default.

Why are the changes needed?

Fixing the compatibility issue with default collation config and collate function.

Does this PR introduce any user-facing change?

Customers will be able to execute collation(, ) function even when default collation config is configured to some other collation than UTF8_BINARY. We are expanding the surface area for cx.

How was this patch tested?

Added tests.

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

No.

@github-actions github-actions bot added the SQL label May 14, 2024
Copy link
Contributor

@mihailomilosevic2001 mihailomilosevic2001 left a comment

Choose a reason for hiding this comment

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

LGTM except for few comments

@nebojsa-db nebojsa-db changed the title [SPARK-48263] Collate function not working when default collation config set [SPARK-48263] Collate function support for non UTF8_BINARY strings May 14, 2024
@nebojsa-db
Copy link
Contributor Author

Please take a look :) @uros-db @stefankandic @nikolamand-db

Copy link
Contributor

@uros-db uros-db left a comment

Choose a reason for hiding this comment

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

lgtm

@uros-db
Copy link
Contributor

uros-db commented May 14, 2024

btw, I'd say that this PR does introduce some user-facing changes - so I'd update the PR description to reflect this with more details

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 91da2ca May 14, 2024
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.

5 participants