You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "uros-db (via GitHub)" <gi...@apache.org> on 2024/05/21 09:41:03 UTC
[PR] [WIP][SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
uros-db opened a new pull request, #46682:
URL: https://github.com/apache/spark/pull/46682
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1617954653
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -639,81 +639,93 @@ public void testStringInstr() throws SparkException {
assertStringInstr("abi̇o12", "İo", "UNICODE_CI", 3);
}
- private void assertFindInSet(String word, String set, String collationName,
- Integer expected) throws SparkException {
+ private void assertFindInSet(String word, UTF8String set, String collationName,
+ Integer expected) throws SparkException {
UTF8String w = UTF8String.fromString(word);
- UTF8String s = UTF8String.fromString(set);
int collationId = CollationFactory.collationNameToId(collationName);
- assertEquals(expected, CollationSupport.FindInSet.exec(w, s, collationId));
+ assertEquals(expected, CollationSupport.FindInSet.exec(w, set, collationId));
}
@Test
public void testFindInSet() throws SparkException {
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("abc", "abc,b,ab,c,def", "UTF8_BINARY", 1);
- assertFindInSet("def", "abc,b,ab,c,def", "UTF8_BINARY", 5);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("a", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("c", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 3);
- assertFindInSet("AbC", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("abcd", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("XX", "xx", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("ab", "abc,b,ab,c,def", "UNICODE", 3);
- assertFindInSet("Ab", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("xx", "xx", "UNICODE", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE", 0);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE", 5);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("C", "abc,b,ab,c,def", "UNICODE_CI", 4);
- assertFindInSet("DeF", "abc,b,ab,c,dEf", "UNICODE_CI", 5);
- assertFindInSet("DEFG", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("XX", "xx", "UNICODE_CI", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
- assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
- assertFindInSet("i", "İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
- assertFindInSet("i", "İ,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
- assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("abc", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 1);
+ assertFindInSet("def", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 5);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("c", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 3);
+ assertFindInSet("AbC", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("abcd", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
Review Comment:
Same here please:
```scala
assertFindInSet("", UTF8String.fromString(",abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 1);
assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def,"), "UTF8_BINARY_LCASE", 6);
assertFindInSet("", UTF8String.fromString("abc"), "UTF8_BINARY_LCASE", 0);
```
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -639,81 +639,93 @@ public void testStringInstr() throws SparkException {
assertStringInstr("abi̇o12", "İo", "UNICODE_CI", 3);
}
- private void assertFindInSet(String word, String set, String collationName,
- Integer expected) throws SparkException {
+ private void assertFindInSet(String word, UTF8String set, String collationName,
+ Integer expected) throws SparkException {
UTF8String w = UTF8String.fromString(word);
- UTF8String s = UTF8String.fromString(set);
int collationId = CollationFactory.collationNameToId(collationName);
- assertEquals(expected, CollationSupport.FindInSet.exec(w, s, collationId));
+ assertEquals(expected, CollationSupport.FindInSet.exec(w, set, collationId));
}
@Test
public void testFindInSet() throws SparkException {
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("abc", "abc,b,ab,c,def", "UTF8_BINARY", 1);
- assertFindInSet("def", "abc,b,ab,c,def", "UTF8_BINARY", 5);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("a", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("c", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 3);
- assertFindInSet("AbC", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("abcd", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("XX", "xx", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("ab", "abc,b,ab,c,def", "UNICODE", 3);
- assertFindInSet("Ab", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("xx", "xx", "UNICODE", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE", 0);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE", 5);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("C", "abc,b,ab,c,def", "UNICODE_CI", 4);
- assertFindInSet("DeF", "abc,b,ab,c,dEf", "UNICODE_CI", 5);
- assertFindInSet("DEFG", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("XX", "xx", "UNICODE_CI", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
- assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
- assertFindInSet("i", "İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
- assertFindInSet("i", "İ,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
- assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("abc", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 1);
+ assertFindInSet("def", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 5);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("c", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 3);
+ assertFindInSet("AbC", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("abcd", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 3);
+ assertFindInSet("Ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("xx", UTF8String.fromString("xx"), "UNICODE", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 0);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 5);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("C", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 4);
+ assertFindInSet("DeF", UTF8String.fromString("abc,b,ab,c,dEf"), "UNICODE_CI", 5);
+ assertFindInSet("DEFG", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UNICODE_CI", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 4);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,界Xx,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UNICODE_CI", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UNICODE_CI", 2);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UTF8_BINARY_LCASE", 2);
+ // Invalid UTF8 strings
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY", 3);
+ assertFindInSet("c", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UNICODE", 3);
Review Comment:
I think I understand. It is probably related to the fix in this PR: https://github.com/apache/spark/pull/46772
Could we add a TODO item or merge the other PR and then fix this one?
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -639,81 +639,93 @@ public void testStringInstr() throws SparkException {
assertStringInstr("abi̇o12", "İo", "UNICODE_CI", 3);
}
- private void assertFindInSet(String word, String set, String collationName,
- Integer expected) throws SparkException {
+ private void assertFindInSet(String word, UTF8String set, String collationName,
+ Integer expected) throws SparkException {
UTF8String w = UTF8String.fromString(word);
- UTF8String s = UTF8String.fromString(set);
int collationId = CollationFactory.collationNameToId(collationName);
- assertEquals(expected, CollationSupport.FindInSet.exec(w, s, collationId));
+ assertEquals(expected, CollationSupport.FindInSet.exec(w, set, collationId));
}
@Test
public void testFindInSet() throws SparkException {
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("abc", "abc,b,ab,c,def", "UTF8_BINARY", 1);
- assertFindInSet("def", "abc,b,ab,c,def", "UTF8_BINARY", 5);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("a", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("c", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 3);
- assertFindInSet("AbC", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("abcd", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("XX", "xx", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("ab", "abc,b,ab,c,def", "UNICODE", 3);
- assertFindInSet("Ab", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("xx", "xx", "UNICODE", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE", 0);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE", 5);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("C", "abc,b,ab,c,def", "UNICODE_CI", 4);
- assertFindInSet("DeF", "abc,b,ab,c,dEf", "UNICODE_CI", 5);
- assertFindInSet("DEFG", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("XX", "xx", "UNICODE_CI", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
- assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
- assertFindInSet("i", "İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
- assertFindInSet("i", "İ,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
- assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("abc", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 1);
+ assertFindInSet("def", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 5);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
Review Comment:
Can we also add these two test cases please?
```scala
assertFindInSet("", UTF8String.fromString(",abc,b,ab,c,def"), "UTF8_BINARY", 1);
assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def,"), "UTF8_BINARY", 6);
assertFindInSet("", UTF8String.fromString("abc"), "UTF8_BINARY", 0);
```
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -639,81 +639,93 @@ public void testStringInstr() throws SparkException {
assertStringInstr("abi̇o12", "İo", "UNICODE_CI", 3);
}
- private void assertFindInSet(String word, String set, String collationName,
- Integer expected) throws SparkException {
+ private void assertFindInSet(String word, UTF8String set, String collationName,
+ Integer expected) throws SparkException {
UTF8String w = UTF8String.fromString(word);
- UTF8String s = UTF8String.fromString(set);
int collationId = CollationFactory.collationNameToId(collationName);
- assertEquals(expected, CollationSupport.FindInSet.exec(w, s, collationId));
+ assertEquals(expected, CollationSupport.FindInSet.exec(w, set, collationId));
}
@Test
public void testFindInSet() throws SparkException {
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("abc", "abc,b,ab,c,def", "UTF8_BINARY", 1);
- assertFindInSet("def", "abc,b,ab,c,def", "UTF8_BINARY", 5);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("a", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("c", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 3);
- assertFindInSet("AbC", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("abcd", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("XX", "xx", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("ab", "abc,b,ab,c,def", "UNICODE", 3);
- assertFindInSet("Ab", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("xx", "xx", "UNICODE", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE", 0);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE", 5);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("C", "abc,b,ab,c,def", "UNICODE_CI", 4);
- assertFindInSet("DeF", "abc,b,ab,c,dEf", "UNICODE_CI", 5);
- assertFindInSet("DEFG", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("XX", "xx", "UNICODE_CI", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
- assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
- assertFindInSet("i", "İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
- assertFindInSet("i", "İ,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
- assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("abc", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 1);
+ assertFindInSet("def", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 5);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("c", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 3);
+ assertFindInSet("AbC", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("abcd", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 3);
+ assertFindInSet("Ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("xx", UTF8String.fromString("xx"), "UNICODE", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 0);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 5);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("C", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 4);
+ assertFindInSet("DeF", UTF8String.fromString("abc,b,ab,c,dEf"), "UNICODE_CI", 5);
+ assertFindInSet("DEFG", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UNICODE_CI", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 4);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,界Xx,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UNICODE_CI", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UNICODE_CI", 2);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UTF8_BINARY_LCASE", 2);
+ // Invalid UTF8 strings
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY", 3);
+ assertFindInSet("c", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UNICODE", 3);
Review Comment:
Why do we get 3 here? This seems a bit strange. I would expect the same result as for UNICODE_CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613448325
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int len = 0; len <= endPos; ++len) {
+ if (target.substring(endPos - len, endPos).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the last occurrence of the pattern string in the target string,
+ * ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return the position of the last occurrence of pattern in target
+ */
+ public static int lowercaseRFind(
Review Comment:
this one can go private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on PR #46682:
URL: https://github.com/apache/spark/pull/46682#issuecomment-2128279874
> ### Why are the changes needed?
> Fix functions that give unusable results due to one-to-many conditional case mapping when performing string search under UTF8_BINARY_LCASE (see example above).
Please remove the word "conditional" in the above section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613221638
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -610,8 +610,42 @@ public void testFindInSet() throws SparkException {
assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
+ assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
+ assertFindInSet("i", "İ", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
+ assertFindInSet("i", "i̇", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
+ assertFindInSet("i", "İ,", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
+ assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
+ assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
Review Comment:
returns 3, as expected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1612793743
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int len = 0; len <= endPos; ++len) {
+ if (target.substring(endPos - len, endPos).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the last occurrence of the pattern string in the target string,
+ * ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return the position of the last occurrence of pattern in target
+ */
+ public static int lowercaseRFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int i = endPos; i >= 0; --i) {
+ if (lowercaseMatchUntil(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
Review Comment:
yup, reviewed already - although some access modifiers should likely be changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613404598
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
Review Comment:
this one can go private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613447350
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
Review Comment:
this one can go private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1612519155
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
Review Comment:
Do we need this to be a public method?
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
Review Comment:
Same question.
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -610,8 +610,42 @@ public void testFindInSet() throws SparkException {
assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
+ assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
+ assertFindInSet("i", "İ", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
+ assertFindInSet("i", "i̇", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
+ assertFindInSet("i", "İ,", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
+ assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
+ assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
+ assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
+ assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
Review Comment:
Could you please add as a test case the one that corresponds to this query?
```sql
select find_in_set('c' collate utf8_binary_lcase,cast(unhex('41C22C422C432C56') as string) collate utf8_binary_lcase)
```
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -610,8 +610,42 @@ public void testFindInSet() throws SparkException {
assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
+ assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
Review Comment:
I see tests for FindInSet, which is also mentioned in the PR description. Is it the case that there were no changes needed for the FindInSet implementation?
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
Review Comment:
And for this one.
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int len = 0; len <= endPos; ++len) {
+ if (target.substring(endPos - len, endPos).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the last occurrence of the pattern string in the target string,
+ * ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return the position of the last occurrence of pattern in target
+ */
+ public static int lowercaseRFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int i = endPos; i >= 0; --i) {
+ if (lowercaseMatchUntil(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
Review Comment:
I believe I have reviewed all the above in another PR. Please let me know if there are changes that I need to go over.
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -648,8 +682,22 @@ public void testReplace() throws SparkException {
assertReplace("replace", "", "123", "UNICODE_CI", "replace");
assertReplace("aBc世abc", "b", "12", "UNICODE_CI", "a12c世a12c");
assertReplace("a世Bcdabcd", "bC", "", "UNICODE_CI", "a世dad");
+ assertReplace("abi̇12", "i", "X", "UNICODE_CI", "abi̇12");
+ assertReplace("abi̇12", "\u0307", "X", "UNICODE_CI", "abi̇12");
+ assertReplace("abi̇12", "İ", "X", "UNICODE_CI", "abX12");
+ assertReplace("abİ12", "i", "X", "UNICODE_CI", "abİ12");
+ assertReplace("İi̇İi̇İi̇", "i̇", "x", "UNICODE_CI", "xxxxxx");
+ assertReplace("İi̇İi̇İi̇", "i", "x", "UNICODE_CI", "İi̇İi̇İi̇");
assertReplace("abİo12i̇o", "i̇o", "xx", "UNICODE_CI", "abxx12xx");
assertReplace("abi̇o12i̇o", "İo", "yy", "UNICODE_CI", "abyy12yy");
+ assertReplace("abi̇12", "i", "X", "UTF8_BINARY_LCASE", "abẊ12"); // != UNICODE_CI
+ assertReplace("abi̇12", "\u0307", "X", "UTF8_BINARY_LCASE", "abiX12"); // != UNICODE_CI
+ assertReplace("abi̇12", "İ", "X", "UTF8_BINARY_LCASE", "abX12");
+ assertReplace("abİ12", "i", "X", "UTF8_BINARY_LCASE", "abİ12");
+ assertReplace("İi̇İi̇İi̇", "i̇", "x", "UTF8_BINARY_LCASE", "xxxxxx");
+ assertReplace("İi̇İi̇İi̇", "i", "x", "UTF8_BINARY_LCASE", "İẋİẋİẋ"); // != UNICODE_CI
Review Comment:
```suggestion
assertReplace("İi̇İi̇İi̇", "i", "x", "UTF8_BINARY_LCASE", "İx\u0307İx\u0307İx\u0307"); // != UNICODE_CI
```
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
Review Comment:
Do we need this to be a public method?
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
Review Comment:
Same for this one as well.
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ return lowercaseMatchLengthUntil(target, lowercasePattern, endPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that ends with the specified
+ * suffix, ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * suffix is already lowercased. The method only considers the part of target string that ends
+ * at the specified (non-inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the suffix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return length of the target substring that ends with the specified suffix in lowercase
+ */
+ public static int lowercaseMatchLengthUntil(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int endPos) {
+ assert endPos <= target.numChars();
+ for (int len = 0; len <= endPos; ++len) {
+ if (target.substring(endPos - len, endPos).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the last occurrence of the pattern string in the target string,
+ * ending at the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return the position of the last occurrence of pattern in target
+ */
+ public static int lowercaseRFind(
Review Comment:
And this.
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -648,8 +682,22 @@ public void testReplace() throws SparkException {
assertReplace("replace", "", "123", "UNICODE_CI", "replace");
assertReplace("aBc世abc", "b", "12", "UNICODE_CI", "a12c世a12c");
assertReplace("a世Bcdabcd", "bC", "", "UNICODE_CI", "a世dad");
+ assertReplace("abi̇12", "i", "X", "UNICODE_CI", "abi̇12");
+ assertReplace("abi̇12", "\u0307", "X", "UNICODE_CI", "abi̇12");
+ assertReplace("abi̇12", "İ", "X", "UNICODE_CI", "abX12");
+ assertReplace("abİ12", "i", "X", "UNICODE_CI", "abİ12");
+ assertReplace("İi̇İi̇İi̇", "i̇", "x", "UNICODE_CI", "xxxxxx");
+ assertReplace("İi̇İi̇İi̇", "i", "x", "UNICODE_CI", "İi̇İi̇İi̇");
assertReplace("abİo12i̇o", "i̇o", "xx", "UNICODE_CI", "abxx12xx");
assertReplace("abi̇o12i̇o", "İo", "yy", "UNICODE_CI", "abyy12yy");
+ assertReplace("abi̇12", "i", "X", "UTF8_BINARY_LCASE", "abẊ12"); // != UNICODE_CI
Review Comment:
```suggestion
assertReplace("abi̇12", "i", "X", "UTF8_BINARY_LCASE", "abX\u030712"); // != UNICODE_CI
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613408156
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int i = startPos; i <= target.numChars(); ++i) {
+ if (lowercaseMatchFrom(target, lowercasePattern, i)) {
+ return i;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns whether the target string ends with the specified suffix, ending at the specified
+ * position (0-based index referring to character position in UTF8String), with respect to the
+ * UTF8_BINARY_LCASE collation. The method assumes that the suffix is already lowercased prior
+ * to method call to avoid the overhead of calling .toLowerCase() multiple times on the same
+ * suffix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param endPos the end position for searching (in the target string)
+ * @return whether the target string ends with the specified suffix in lowercase
+ */
+ public static boolean lowercaseMatchUntil(
Review Comment:
yes, this will need to be a public method (changes in https://github.com/apache/spark/pull/46511)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613407352
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ return lowercaseMatchLengthFrom(target, lowercasePattern, startPos) != MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the length of the substring of the target string that starts with the specified
+ * prefix, starting from the specified position (0-based index referring to character position
+ * in UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * prefix is already lowercased. The method only considers the part of target string that
+ * starts from the specified (inclusive) position (that is, the method does not look at UTF8
+ * characters of the target string at or after position `endPos`). If the prefix is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return length of the target substring that ends with the specified prefix in lowercase
+ */
+ public static int lowercaseMatchLengthFrom(
+ final UTF8String target,
+ final UTF8String lowercasePattern,
+ int startPos) {
+ assert startPos >= 0;
+ for (int len = 0; len <= target.numChars() - startPos; ++len) {
+ if (target.substring(startPos, startPos + len).toLowerCase().equals(lowercasePattern)) {
+ return len;
+ }
+ }
+ return MATCH_NOT_FOUND;
+ }
+
+ /**
+ * Returns the position of the first occurrence of the pattern string in the target string,
+ * starting from the specified position (0-based index referring to character position in
+ * UTF8String), with respect to the UTF8_BINARY_LCASE collation. The method assumes that the
+ * pattern string is already lowercased prior to call. If the pattern is not found,
+ * MATCH_NOT_FOUND is returned.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return the position of the first occurrence of pattern in target
+ */
+ public static int lowercaseFind(
Review Comment:
this one can go private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1612791411
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -610,8 +610,42 @@ public void testFindInSet() throws SparkException {
assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
+ assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
Review Comment:
yes, there were luckily no implementation changes required for FindInSet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1613401443
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java:
##########
@@ -34,6 +34,155 @@
* Utility class for collation-aware UTF8String operations.
*/
public class CollationAwareUTF8String {
+
+ /**
+ * The constant value to indicate that the match is not found when searching for a pattern
+ * string in a target string.
+ */
+ private static final int MATCH_NOT_FOUND = -1;
+
+ /**
+ * Returns whether the target string starts with the specified prefix, starting from the
+ * specified position (0-based index referring to character position in UTF8String), with respect
+ * to the UTF8_BINARY_LCASE collation. The method assumes that the prefix is already lowercased
+ * prior to method call to avoid the overhead of calling .toLowerCase() multiple times on the
+ * same prefix string.
+ *
+ * @param target the string to be searched in
+ * @param lowercasePattern the string to be searched for
+ * @param startPos the start position for searching (in the target string)
+ * @return whether the target string starts with the specified prefix in UTF8_BINARY_LCASE
+ */
+ public static boolean lowercaseMatchFrom(
Review Comment:
yes, this will need to be a public method (changes in https://github.com/apache/spark/pull/46511)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48282][SQL] Alter string search logic for UTF8_BINARY_LCASE collation (StringReplace, FindInSet) [spark]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #46682:
URL: https://github.com/apache/spark/pull/46682#discussion_r1618206224
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -639,81 +639,93 @@ public void testStringInstr() throws SparkException {
assertStringInstr("abi̇o12", "İo", "UNICODE_CI", 3);
}
- private void assertFindInSet(String word, String set, String collationName,
- Integer expected) throws SparkException {
+ private void assertFindInSet(String word, UTF8String set, String collationName,
+ Integer expected) throws SparkException {
UTF8String w = UTF8String.fromString(word);
- UTF8String s = UTF8String.fromString(set);
int collationId = CollationFactory.collationNameToId(collationName);
- assertEquals(expected, CollationSupport.FindInSet.exec(w, s, collationId));
+ assertEquals(expected, CollationSupport.FindInSet.exec(w, set, collationId));
}
@Test
public void testFindInSet() throws SparkException {
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("abc", "abc,b,ab,c,def", "UTF8_BINARY", 1);
- assertFindInSet("def", "abc,b,ab,c,def", "UTF8_BINARY", 5);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY", 0);
- assertFindInSet("a", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("c", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("AB", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 3);
- assertFindInSet("AbC", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("abcd", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("XX", "xx", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("", "abc,b,ab,c,def", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UTF8_BINARY_LCASE", 4);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("ab", "abc,b,ab,c,def", "UNICODE", 3);
- assertFindInSet("Ab", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("d,ef", "abc,b,ab,c,def", "UNICODE", 0);
- assertFindInSet("xx", "xx", "UNICODE", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE", 0);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE", 5);
- assertFindInSet("a", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("C", "abc,b,ab,c,def", "UNICODE_CI", 4);
- assertFindInSet("DeF", "abc,b,ab,c,dEf", "UNICODE_CI", 5);
- assertFindInSet("DEFG", "abc,b,ab,c,def", "UNICODE_CI", 0);
- assertFindInSet("XX", "xx", "UNICODE_CI", 1);
- assertFindInSet("界x", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 4);
- assertFindInSet("界x", "test,大千,界Xx,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("大", "test,大千,世,界X,大,千,世界", "UNICODE_CI", 5);
- assertFindInSet("i̇", "İ", "UNICODE_CI", 1);
- assertFindInSet("i", "İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "İ,", "UNICODE_CI", 1);
- assertFindInSet("i", "İ,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "i̇,", "UNICODE_CI", 1);
- assertFindInSet("i", "i̇,", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,İ,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,İ,12", "UNICODE_CI", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UNICODE_CI", 2);
- assertFindInSet("i", "ab,i̇,12", "UNICODE_CI", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UNICODE_CI", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UNICODE_CI", 2);
- assertFindInSet("i̇", "İ", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "İ,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "İ,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "i̇,", "UTF8_BINARY_LCASE", 1);
- assertFindInSet("i", "i̇,", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,İ,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,İ,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇", "ab,i̇,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("i", "ab,i̇,12", "UTF8_BINARY_LCASE", 0);
- assertFindInSet("i̇o", "ab,İo,12", "UTF8_BINARY_LCASE", 2);
- assertFindInSet("İo", "ab,i̇o,12", "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("abc", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 1);
+ assertFindInSet("def", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 5);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY", 0);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("c", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("AB", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 3);
+ assertFindInSet("AbC", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("abcd", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("", UTF8String.fromString("abc,b,ab,c,def"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UTF8_BINARY_LCASE", 4);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 3);
+ assertFindInSet("Ab", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("d,ef", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE", 0);
+ assertFindInSet("xx", UTF8String.fromString("xx"), "UNICODE", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 0);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE", 5);
+ assertFindInSet("a", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("C", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 4);
+ assertFindInSet("DeF", UTF8String.fromString("abc,b,ab,c,dEf"), "UNICODE_CI", 5);
+ assertFindInSet("DEFG", UTF8String.fromString("abc,b,ab,c,def"), "UNICODE_CI", 0);
+ assertFindInSet("XX", UTF8String.fromString("xx"), "UNICODE_CI", 1);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 4);
+ assertFindInSet("界x", UTF8String.fromString("test,大千,界Xx,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("大", UTF8String.fromString("test,大千,世,界X,大,千,世界"), "UNICODE_CI", 5);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UNICODE_CI", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UNICODE_CI", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UNICODE_CI", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UNICODE_CI", 2);
+ assertFindInSet("i̇", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("İ,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 1);
+ assertFindInSet("i", UTF8String.fromString("i̇,"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,İ,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("i", UTF8String.fromString("ab,i̇,12"), "UTF8_BINARY_LCASE", 0);
+ assertFindInSet("i̇o", UTF8String.fromString("ab,İo,12"), "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("İo", UTF8String.fromString("ab,i̇o,12"), "UTF8_BINARY_LCASE", 2);
+ // Invalid UTF8 strings
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY", 3);
+ assertFindInSet("c", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UTF8_BINARY_LCASE", 2);
+ assertFindInSet("C", UTF8String.fromBytes(
+ new byte[] { 0x41, (byte) 0xC2, 0x2C, 0x42, 0x2C, 0x43, 0x2C, 0x43, 0x2C, 0x56 }),
+ "UNICODE", 3);
Review Comment:
yup, we'll merge https://github.com/apache/spark/pull/46772 first, and then this will need to get fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org