srielau commented on code in PR #58132:
URL: https://github.com/apache/spark/pull/58132#discussion_r3815985442
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/util/JdbcTypeUtils.scala:
##########
@@ -97,6 +99,8 @@ private[jdbc] object JdbcTypeUtils {
case LongType => 19
case FloatType => 7
case DoubleType => 15
+ case c: CharType => c.length
+ case v: VarcharType => v.length
Review Comment:
Addressed in 4673559.
`CHAR_OCTET_LENGTH` now reports the declared length `n` for CHAR/VARCHAR
(STRING stays 0). `getTypeInfo` lists CHAR and VARCHAR with `CREATE_PARAMS =
length`, ordered by DATA_TYPE then TYPE_NAME. Added a table `getColumns` test
covering type code, type name, COLUMN_SIZE, and CHAR_OCTET_LENGTH.
##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectJdbcDataTypeSuite.scala:
##########
@@ -223,6 +223,36 @@ class SparkConnectJdbcDataTypeSuite extends
ConnectFunSuite with RemoteSparkSess
}
}
+ test("SPARK-58794: get char and varchar types") {
Review Comment:
Added `SPARK-58794: getColumns for CHAR/VARCHAR table` in this suite.
Connect JDBC suites still fail to compile locally (`CheckReturnValue`); relying
on GitHub CI for the run.
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkGetColumnsOperation.scala:
##########
@@ -136,6 +136,7 @@ private[hive] class SparkGetColumnsOperation(
CalendarIntervalType | NullType | _: AnsiIntervalType) =>
Some(dt.defaultSize)
case c: CharType => Some(c.length)
+ case v: VarcharType => Some(v.length)
Review Comment:
Scaladoc updated. HS2 `CHAR_OCTET_LENGTH` now returns `n` for CHAR/VARCHAR
and stays null otherwise, matching Connect's filled-in width (Connect uses 0
for N/A).
##########
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SparkMetadataOperationSuite.scala:
##########
@@ -338,7 +338,7 @@ class SparkMetadataOperationSuite extends
HiveThriftServer2TestBase {
val colSize = rowSet.getInt("COLUMN_SIZE")
schema(pos).dataType match {
- case StringType | BinaryType | _: ArrayType | _: MapType | _:
VarcharType =>
+ case StringType | BinaryType | _: ArrayType | _: MapType =>
assert(colSize === 0)
case o => assert(colSize === o.defaultSize)
Review Comment:
Matching `CharType` / `VarcharType` on `.length` now, with explicit 255 /
1024 for c17 / c18. The table test also enables `standardSemantics`.
##########
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SparkMetadataOperationSuite.scala:
##########
@@ -372,6 +372,23 @@ class SparkMetadataOperationSuite extends
HiveThriftServer2TestBase {
}
}
+ test("SPARK-58794: result metadata preserves CHAR and VARCHAR") {
Review Comment:
c17 / c18 `COLUMN_SIZE` (and `CHAR_OCTET_LENGTH`) are asserted in the
`getColumns` test above. This CAST result-metadata test stays as the Hive JDBC
name pin (`char` / `varchar`).
--
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]
