Skip to content

[SPARK-47359][SQL] Support TRANSLATE function to work with collated strings#45820

Closed
miland-db wants to merge 30 commits into
apache:masterfrom
miland-db:miland-db/string-translate
Closed

[SPARK-47359][SQL] Support TRANSLATE function to work with collated strings#45820
miland-db wants to merge 30 commits into
apache:masterfrom
miland-db:miland-db/string-translate

Conversation

@miland-db

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Extend built-in string functions to support non-binary, non-lowercase collation for: translate

Why are the changes needed?

Update collation support for built-in string functions in Spark.

Does this PR introduce any user-facing change?

Yes, users should now be able to use COLLATE within arguments for built-in string function TRANSLATE in Spark SQL queries, using non-binary collations such as UNICODE_CI.

How was this patch tested?

Unit tests for queries using StringTranslate (CollationStringExpressionsSuite.scala).

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

No

@github-actions github-actions Bot added the SQL label Apr 2, 2024
@miland-db

Copy link
Copy Markdown
Contributor Author

Adding Belgrade collation crew if anyone would like to review this: @uros-db @stefankandic @mihailom-db @dbatomic

Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
Comment thread common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java Outdated
@HyukjinKwon

Copy link
Copy Markdown
Member

Let's follow https://github.com/databricks/scala-style-guide, and remove unrelated changes, e.g., adding newlines which makes cherry-pick/backporting/reverting difficult.

@uros-db

uros-db commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

heads up: we’ve done some major code restructuring in #45978, so please sync these changes before moving on

@miland-db you’ll likely need to rewrite the code in this PR, so please follow the guidelines outlined in https://issues.apache.org/jira/browse/SPARK-47410

# Conflicts:
#	common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java
#	sql/core/src/test/scala/org/apache/spark/sql/CollationStringExpressionsSuite.scala
# Conflicts:
#	sql/core/src/test/scala/org/apache/spark/sql/CollationStringExpressionsSuite.scala
# Conflicts:
#	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCasts.scala
# Conflicts:
#	common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationSupport.java

@uros-db uros-db left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just flagging this PR will likely need a fix for the ICU implementation

# Conflicts:
#	common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationSupport.java
#	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CollationTypeCasts.scala
# Conflicts:
#	common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationSupport.java

@uros-db uros-db left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@miland-db miland-db requested a review from uros-db April 30, 2024 13:46
@miland-db

Copy link
Copy Markdown
Contributor Author

@cloud-fan please review

@cloud-fan

Copy link
Copy Markdown
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 0329479 Apr 30, 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.

6 participants