stefankandic commented on code in PR #48737:
URL: https://github.com/apache/spark/pull/48737#discussion_r1826332674
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -394,7 +395,11 @@ private static Collation fetchCollation(int collationId) {
// instance.
assert (collationId >= 0 && getDefinitionOrigin(collationId)
== DefinitionOrigin.PREDEFINED);
- if (collationId == UTF8_BINARY_COLLATION_ID) {
+ if (collationId == NON_COLLATED_STRING_COLLATION_ID) {
+ // Skip cache.
+ return CollationSpecUTF8.NON_COLLATED_STRING_COLLATION;
+ }
+ else if (collationId == UTF8_BINARY_COLLATION_ID) {
Review Comment:
Do we even need the new collation spec? Why can't we just return the utf8
binary one? The idea is that these two should behave identically anyways
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -394,7 +395,11 @@ private static Collation fetchCollation(int collationId) {
// instance.
assert (collationId >= 0 && getDefinitionOrigin(collationId)
== DefinitionOrigin.PREDEFINED);
- if (collationId == UTF8_BINARY_COLLATION_ID) {
Review Comment:
can we call these it `IMPLICIT_UTF8_BINARY_COLLATION_ID` and
`EXPLICIT_UTF8_BINARY_COLLATION_ID`?
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StringType.scala:
##########
@@ -74,7 +77,7 @@ class StringType private (val collationId: Int) extends
AtomicType with Serializ
* `string` due to backwards compatibility.
*/
override def typeName: String =
- if (isUTF8BinaryCollation) "string"
+ if (isNonCollatedString) "string"
Review Comment:
i don't think we should change this
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
