cloud-fan commented on code in PR #58132:
URL: https://github.com/apache/spark/pull/58132#discussion_r3818524418
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/util/JdbcTypeUtils.scala:
##########
@@ -168,6 +172,17 @@ private[jdbc] object JdbcTypeUtils {
case _ => null
}
+ /**
+ * JDBC `CHAR_OCTET_LENGTH`. Spark CHAR/VARCHAR lengths are in characters;
report `n` so
+ * clients that size buffers from this column see the declared width instead
of 0.
+ * Unbounded STRING and non-character types keep 0 (not applicable /
unknown).
+ */
+ def getCharOctetLength(field: StructField): Int = field.dataType match {
+ case c: CharType => c.length
Review Comment:
**Blocking:**
`CHAR_OCTET_LENGTH` is byte capacity, but this returns character count. A
`CHAR(4)` value can contain four multibyte characters, so clients sizing a
buffer from this value can truncate it. Please report the maximum UTF-8 byte
length with overflow handling, or leave the field unknown; apply the same
correction to the HiveServer2 helper.
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -72,14 +72,31 @@ object RowEncoder extends DataTypeErrorsBase {
encoderForDataType(schema, lenient).asInstanceOf[AgnosticEncoder[Row]]
}
+ /**
+ * Builds an encoder for a schema that the engine produced, such as the
result schema of a Spark
+ * Connect query. Whether CHAR/VARCHAR are first class types is decided by
the session that
Review Comment:
**Nit:**
```suggestion
* Connect query. Whether CHAR/VARCHAR are first-class types is decided by
the session that
```
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkGetColumnsOperation.scala:
##########
@@ -126,16 +126,18 @@ private[hive] class SparkGetColumnsOperation(
}
/**
- * For boolean, numeric and datetime types, it returns the default size of
its catalyst type
+ * For boolean, numeric and datetime types, it returns the default size of
its catalyst type.
+ * For CHAR(n) and VARCHAR(n), it returns the declared character length n.
* For struct type, when its elements are fixed-size, the summation of all
element sizes will be
* returned.
- * For array, map, string, and binaries, the column size is variable, return
null as unknown.
+ * For array, map, unbounded string, and binaries, the column size is
variable, return null.
Review Comment:
**Nit:**
```suggestion
* For array, map, unbounded string, and binaries, the column size is
variable; return null.
```
--
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]