[SPARK-48435][SQL] UNICODE collation should not support binary equality #46772
Conversation
| @@ -245,12 +245,12 @@ public CollationIdentifier identifier() { | |||
| PROVIDER_ICU, | |||
| Collator.getInstance(ULocale.ROOT), | |||
| "153.120.0.0", | |||
| true, | |||
| false, | |||
There was a problem hiding this comment.
note for reviewers: this is the place we say UNICODE does not supportBinaryEquality
|
another note for future reference: as per ICU docs, we've decided to stick with TERTIARY collation strength for UNICODE |
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java
Show resolved
Hide resolved
common/unsafe/src/test/scala/org/apache/spark/unsafe/types/CollationFactorySuite.scala
Outdated
Show resolved
Hide resolved
common/unsafe/src/test/scala/org/apache/spark/unsafe/types/CollationFactorySuite.scala
Outdated
Show resolved
Hide resolved
|
As noted in one of the resolved comments - there's no loss of coverage. However, some tests have been (temporarily) removed because At this time, |
...lyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollationExpressionSuite.scala
Show resolved
Hide resolved
This explanation would fit very nicely in the PR description (explaining what are the changes in the PR). Your call. |
uros-db
changed the title
|
updated PR title and description |
|
thanks, merging to master! |
What changes were proposed in this pull request?
CollationFactory has been updated to no longer mark UNICODE as collation that supportsBinaryCollation. To reflect these changes, various tests have been updated.
However, some tests have been (temporarily) removed because StringTrim no longer supports UNICODE collation given the new UNICODE definition in CollationFactory. At this time, StringTrim expression only supports UTF8_BINARY & UTF8_BINARY_LCASE, but not ICU collations. This work is in progress (#46762), so we'll ensure appropriate test coverage with those changes.
Why are the changes needed?
UNICODE collation should not support binary collation. Note: in the future, we may want to consider a collation such as UNICODE_BINARY, which will support binary equality, but also maintain UNICODE ordering.
Does this PR introduce any user-facing change?
Yes, UNICODE is no longer treated as a binary collation. This affects how equality works for UNICODE, and also which codepath is taken for various collation-aware string expression given UNICODE collated string arguments.
How was this patch tested?
Updated existing unit and e2e sql test for UNICODE collation.
Was this patch authored or co-authored using generative AI tooling?
No.