You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "stefankandic (via GitHub)" <gi...@apache.org> on 2025/01/09 12:19:56 UTC
[PR] [SPARK-50774] Make collation names public in CollationFactory [spark]
stefankandic opened a new pull request, #49425:
URL: https://github.com/apache/spark/pull/49425
### What changes were proposed in this pull request?
Making collation names public.
### Why are the changes needed?
To be able to have this centralized and not have to create new string literals with "UTF8_BINARY" over and over again.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Should be covered with existing tests.
### 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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909116747
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5773,7 +5773,8 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
def trimCollationEnabled: Boolean = getConf(TRIM_COLLATION_ENABLED)
override def defaultStringType: StringType = {
- if (getConf(DEFAULT_COLLATION).toUpperCase(Locale.ROOT) == "UTF8_BINARY") {
+ if (getConf(DEFAULT_COLLATION).toUpperCase(Locale.ROOT) ==
+ CollationFactory.UTF8_BINARY_COLLATION_NAME) {
Review Comment:
nit, maybe, two more spaces for indentation?
--
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-50774][SQL] Centralize collation names in one place [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #49425: [SPARK-50774][SQL] Centralize collation names in one place
URL: https://github.com/apache/spark/pull/49425
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "stefankandic (via GitHub)" <gi...@apache.org>.
stefankandic commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2580829914
> Oh, if that is only a single file (containing most of them), could you include `CollationSupportSuite` into this PR?
Done
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "stefankandic (via GitHub)" <gi...@apache.org>.
stefankandic commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2580797873
> +1, the proposal sounds reasonable and this PR handles all instances in non-test code.
>
> > To be able to have this centralized and not have to create new string literals with "UTF8_BINARY" over and over again.
>
> BTW, I'm just wondering if you happen to know a way which we can remove the repetition completely from the test code too.
>
> ```
> $ git grep "\"UTF8_BINARY\"" | grep Collation | wc -l
> 992
> ```
>
> Thank you, @stefankandic .
There is one suite accounting for almost 700 of those (CollationSupportSuite), so ideally we should refactor it.
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "stefankandic (via GitHub)" <gi...@apache.org>.
stefankandic commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909149706
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5773,7 +5773,8 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
def trimCollationEnabled: Boolean = getConf(TRIM_COLLATION_ENABLED)
override def defaultStringType: StringType = {
- if (getConf(DEFAULT_COLLATION).toUpperCase(Locale.ROOT) == "UTF8_BINARY") {
+ if (getConf(DEFAULT_COLLATION).toUpperCase(Locale.ROOT) ==
+ CollationFactory.UTF8_BINARY_COLLATION_NAME) {
Review Comment:
done!
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909174155
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -1113,6 +1113,13 @@ public CollationIdentifier identifier() {
public static final List<String> SUPPORTED_PROVIDERS = List.of(PROVIDER_SPARK, PROVIDER_ICU);
public static final String COLLATION_PAD_ATTRIBUTE = "NO_PAD";
+ public static final String UTF8_BINARY_COLLATION_NAME =
+ Collation.CollationSpecUTF8.UTF8_BINARY_COLLATION_NAME;
+ public static final String UTF8_LCASE_COLLATION_NAME =
+ Collation.CollationSpecUTF8.UTF8_LCASE_COLLATION_NAME;
+ public static final String UNICODE_COLLATION_NAME = "UNICODE";
+ public static final String UNICODE_CI_COLLATION_NAME = "UNICODE_CI";
Review Comment:
Thank you. BTW, this is added too?
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909253274
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -19,6 +19,10 @@
import org.apache.spark.SparkException;
import org.apache.spark.sql.catalyst.util.CollationAwareUTF8String;
import org.apache.spark.sql.catalyst.util.CollationFactory;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_CI_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_BINARY_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_LCASE_COLLATION_NAME;
Review Comment:
Did this pass `lint-java` too?
I thought all `import static` should go to the bottom like `import static org.junit.jupiter.api.Assertions.*;`.
--
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-50774][SQL] Centralize collation names in one place [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2588846732
Merged to master.
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2583195943
+1 for the above Max's suggestion too
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "stefankandic (via GitHub)" <gi...@apache.org>.
stefankandic commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909177181
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -1113,6 +1113,13 @@ public CollationIdentifier identifier() {
public static final List<String> SUPPORTED_PROVIDERS = List.of(PROVIDER_SPARK, PROVIDER_ICU);
public static final String COLLATION_PAD_ATTRIBUTE = "NO_PAD";
+ public static final String UTF8_BINARY_COLLATION_NAME =
+ Collation.CollationSpecUTF8.UTF8_BINARY_COLLATION_NAME;
+ public static final String UTF8_LCASE_COLLATION_NAME =
+ Collation.CollationSpecUTF8.UTF8_LCASE_COLLATION_NAME;
+ public static final String UNICODE_COLLATION_NAME = "UNICODE";
+ public static final String UNICODE_CI_COLLATION_NAME = "UNICODE_CI";
Review Comment:
Yes, those are the 4 most used collations so especially if we wanted to refactor that suite we should centralize the names for all of those.
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "stefankandic (via GitHub)" <gi...@apache.org>.
stefankandic commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1909278319
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -19,6 +19,10 @@
import org.apache.spark.SparkException;
import org.apache.spark.sql.catalyst.util.CollationAwareUTF8String;
import org.apache.spark.sql.catalyst.util.CollationFactory;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_CI_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_BINARY_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_LCASE_COLLATION_NAME;
Review Comment:
it passed java lint 😄 , but I moved it to the bottom now
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2580813643
Oh, if that is only a single file, could you include `CollationSupportSuite` into this PR?
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49425:
URL: https://github.com/apache/spark/pull/49425#issuecomment-2580896568
Could you run `dev/lint-java` and fix all `LineLength` errors? Otherwise, the CI will fail due to this.
```
$ dev/lint-java
Using `mvn` from path: /Users/dongjoon/APACHE/spark-merge/build/apache-maven-3.9.9/bin/mvn
Using SPARK_LOCAL_IP=localhost
Checkstyle checks failed at following occurrences:
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[162] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[163] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[169] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[170] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1132] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1133] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1135] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1139] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1142] (sizes) LineLength: Line is longer than 100 characters (found 104).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1143] (sizes) LineLength: Line is longer than 100 characters (found 107).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1523] (sizes) LineLength: Line is longer than 100 characters (found 107).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1784] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1785] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1787] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1788] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1789] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1791] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1792] (sizes) LineLength: Line is longer than 100 characters (found 107).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1793] (sizes) LineLength: Line is longer than 100 characters (found 106).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1794] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1795] (sizes) LineLength: Line is longer than 100 characters (found 106).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1857] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1858] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[1860] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2075] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2076] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2622] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2624] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2630] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2632] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2634] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2636] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2642] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2644] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2647] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[2649] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3734] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3735] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3737] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3750] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3751] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3753] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3754] (sizes) LineLength: Line is longer than 100 characters (found 104).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3755] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3757] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3800] (sizes) LineLength: Line is longer than 100 characters (found 112).
[ERROR] src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:[3801] (sizes) LineLength: Line is longer than 100 characters (found 105).
```
In addition, shall we reuse the static constants by `import` instead of defining new variables like `private final String`?
```
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_CI_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UNICODE_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_BINARY_COLLATION_NAME;
+import static org.apache.spark.sql.catalyst.util.CollationFactory.UTF8_LCASE_COLLATION_NAME;
```
```
- private final String UTF8_BINARY_COLLATION_NAME = CollationFactory.UTF8_BINARY_COLLATION_NAME;
- private final String UTF8_LCASE_COLLATION_NAME = CollationFactory.UTF8_LCASE_COLLATION_NAME;
- private final String UNICODE_COLLATION_NAME = CollationFactory.UNICODE_COLLATION_NAME;
- private final String UNICODE_CI_COLLATION_NAME = CollationFactory.UNICODE_CI_COLLATION_NAME;
```
--
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-50774][SQL] Make collation names public in CollationFactory [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #49425:
URL: https://github.com/apache/spark/pull/49425#discussion_r1910196671
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -1113,6 +1113,13 @@ public CollationIdentifier identifier() {
public static final List<String> SUPPORTED_PROVIDERS = List.of(PROVIDER_SPARK, PROVIDER_ICU);
public static final String COLLATION_PAD_ATTRIBUTE = "NO_PAD";
+ public static final String UTF8_BINARY_COLLATION_NAME =
Review Comment:
How about to add new class `CollationNames` with those constants to `org.apache.spark.sql.catalyst.util`:
```java
public class CollationNames {
public static final String UTF8_BINARY = ...
```
BTW, the suffix `_COLLATION_NAME` could be eliminated I believe.
--
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