srielau commented on code in PR #58317:
URL: https://github.com/apache/spark/pull/58317#discussion_r3887233560
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:
##########
@@ -437,6 +437,9 @@ object OrcUtils extends Logging {
s"array<${getOrcSchemaString(a.elementType)}>"
case m: MapType =>
s"map<${getOrcSchemaString(m.keyType)},${getOrcSchemaString(m.valueType)}>"
+ // Keep Spark responsible for CHAR/VARCHAR assignment and scan checks.
Native ORC
+ // CHAR/VARCHAR would truncate or pad before Spark can validate the
original value.
+ case _: CharType | _: VarcharType => StringType.catalogString
Review Comment:
Addressed in 46e1980b947. `OrcDeserializer` now matches all `StringType`
subtypes, and its existing recursive struct/array/map writers carry that
support into nested positions. `getOrcSchemaString` requests physical ORC
STRING only under `standardSemantics`; preserve-only mode retains native
CHAR/VARCHAR enforcement. The regression matrix covers V1/V2 and vectorized/row
readers, and verifies preserve-only `VARCHAR(4)` still yields `abcd` for
`abcdef`. The targeted SPARK-58814 test passes.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1697,33 +1697,94 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
sql("DROP TEMPORARY FUNCTION IF EXISTS std_char_param")
sql("DROP TEMPORARY FUNCTION IF EXISTS std_varchar_param")
}
+ }
+ }
- // ORC catalog tables stamp the catalyst type so typeof survives
write/read.
- withTable("std_orc") {
- sql("CREATE TABLE std_orc (c CHAR(5), v VARCHAR(5)) USING orc")
- sql("INSERT INTO std_orc VALUES ('ab', 'cd')")
- assert(spark.table("std_orc").schema.map(_.dataType) ===
- Seq(CharType(5), VarcharType(5)))
- checkAnswer(
- sql("SELECT concat('<', c, '>'), concat('<', v, '>') FROM std_orc"),
- Row("<ab >", "<cd>"))
+ test("SPARK-58814: major formats preserve CHAR/VARCHAR schemas and values") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ Seq("parquet", "orc").foreach { format =>
+ Seq("v1" -> format, "v2" -> "").foreach { case (sourceVersion,
useV1List) =>
+ withSQLConf(SQLConf.USE_V1_SOURCE_LIST.key -> useV1List) {
+ withTempPath { dir =>
+ val path = dir.getCanonicalPath
+ val input = spark.range(1).selectExpr(
+ "cast('ab' AS CHAR(4)) AS c",
+ "cast('xy' AS VARCHAR(3)) AS v",
+ "named_struct('c', cast('z' AS CHAR(2))) AS s",
+ "array(cast('q' AS VARCHAR(2))) AS a",
+ "map(cast('k' AS CHAR(2)), cast('v' AS VARCHAR(2))) AS m")
+ input.write.mode("overwrite").format(format).save(path)
+
+ val readBack = spark.read.format(format).load(path)
+ assert(DataType.equalsIgnoreNullability(readBack.schema,
input.schema),
+ s"$format $sourceVersion lost CHAR/VARCHAR schema")
+ checkAnswer(
+ readBack.selectExpr("concat('<', c, '>')", "v", "concat('<',
s.c, '>')"),
Review Comment:
Addressed in 46e1980b947. The inferred round-trip assertion now materializes
both `a` and `m`. Added explicit-schema overflow cases for nested struct
(`s.c`), array element (`a`), map key (`mk`), and map value (`mv`), each
checked for `EXCEED_LIMIT_LENGTH` across ORC V1/V2 and vectorized/row readers.
The targeted SPARK-58814 test passes.
--
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]
