ilicmarkodb commented on code in PR #54159:
URL: https://github.com/apache/spark/pull/54159#discussion_r2792635562
##########
sql/connect/common/src/main/scala/org/apache/spark/sql/connect/common/DataTypeProtoConverter.scala:
##########
@@ -214,13 +214,15 @@ object DataTypeProtoConverter {
// StringType must be matched after CharType and VarcharType
case s: StringType =>
+ val stringBuilder = proto.DataType.String.newBuilder()
+ // Send collation only for explicit collations (including explicit
UTF8_BINARY).
+ // Default STRING (case object) has no explicit collation and should
omit it.
+ if (!s.eq(StringType)) {
+
stringBuilder.setCollation(CollationFactory.fetchCollation(s.collationId).collationName)
+ }
proto.DataType
.newBuilder()
- .setString(
- proto.DataType.String
- .newBuilder()
-
.setCollation(CollationFactory.fetchCollation(s.collationId).collationName)
- .build())
+ .setString(stringBuilder.build())
Review Comment:
Without this change, a user who uses JDBC, for example, and doesn't care
about collations would suddenly get `COLLATE UTF8_BINARY`, as in this test case:
https://github.com/apache/spark/blob/9c45ce4f1220650414eaf737c02780bcd6f93913/sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectJdbcDataTypeSuite.scala#L216
--
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]
