Re: [PR] [SPARK-58806][SQL][CONNECT] Expose CHAR/VARCHAR metadata and decode result schemas [spark]

["srielau (via GitHub)" <gi...@apache.org>]

srielau commented on code in PR #58132:
URL: https://github.com/apache/spark/pull/58132#discussion_r3818586624


##########
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:
   Agreed -- JDBC `CHAR_OCTET_LENGTH` is a byte capacity. Spark `n` is characters, so both Connect and HiveServer2 now report `4 * n` (UTF-8 maximum bytes per character), saturating at `Int.MaxValue`. Unbounded STRING stays 0 / null.
   
   Tests: Connect `CHAR(4)` / `VARCHAR(6)` assert 16 / 24; HS2 asserts `length * 4`.
   
   `3e5a346`.



##########
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:
   Applied in `3e5a346`.



##########
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:
   Applied in `3e5a346`.



-- 
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