[SPARK-58814][SQL] Cover CHAR/VARCHAR round-trips and stop ORC truncating scans - #58317
[SPARK-58814][SQL] Cover CHAR/VARCHAR round-trips and stop ORC truncating scans#58317srielau wants to merge 2 commits into
Conversation
…ting scans Keep Spark responsible for CHAR/VARCHAR assignment and scan checks: ORC was mapping an explicit CHAR/VARCHAR read schema to native ORC types, which truncated oversize values before Spark could reject them. Add Parquet/ORC/CSV/Avro coverage for nested inference, user schemas, and INSERT assignment under standardSemantics.
cloud-fan
left a comment
cloud-fan
left a comment
There was a problem hiding this comment.
Please address the non-vectorized ORC decoder incompatibility and preserve-only VARCHAR enforcement, then make the nested ORC regression test materialize the collection fields it introduces.
This mapping is unconditional, so it also runs when preserveCharVarcharTypeInfo=true is the only reason the constrained type is retained and standard semantics is disabled. That branch of addPaddingForScan installs no VARCHAR read-side check. Requesting ORC string therefore removes native varchar(n) enforcement and can expose abcdef under a VARCHAR(4) schema. Please preserve the native constrained schema in preserve-only mode or add equivalent validation, with a value-level regression test for this flag combination.
…cement Decode all string-family types in the row reader, while retaining native ORC constraints in preserve-only mode. Materialize nested collections and verify nested overflow checks across V1/V2 and vectorized/row readers.
cloud-fan
left a comment
cloud-fan
left a comment
There was a problem hiding this comment.
Review summary
Two changes are needed before this is safe to merge: bind ORC reader schema selection to the persisted/analyzed CHAR/VARCHAR semantics so view results cannot silently change with the caller configuration, and make the Avro regression test observe its array and map values.
Findings
2 total: 0 P0, 1 P1, 1 P2, 0 P3.
Blocking (P1)
- Keep the view-bound CHAR/VARCHAR mode for ORC scans —
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:443— see inline.
Non-blocking (P2)
- Assert the Avro array and map values that the test creates —
connector/avro/src/test/scala/org/apache/spark/sql/avro/AvroSuite.scala:3742— see inline.
| // Under standard semantics, keep Spark responsible for CHAR/VARCHAR assignment and scan | ||
| // checks. Native ORC would truncate or pad before Spark can validate the original value. | ||
| // Preserve-only mode retains the native constrained schema and its legacy enforcement. | ||
| case _: CharType | _: VarcharType if SQLConf.get.charVarcharStandardSemantics => |
There was a problem hiding this comment.
Blocking (P1): This reads the task-side caller configuration, but spark.sql.charVarchar.standardSemantics.enabled is persisted with views. If a view is resolved with standard semantics and its caller later disables the setting, this branch requests native ORC VARCHAR; ORC can then truncate abcdef to abcd before the view's already-resolved Spark length check sees the value. That makes the persisted view caller-dependent and silently bypasses EXCEED_LIMIT_LENGTH. Please carry the analyzed/view-bound semantics into ORC reader construction instead, and add a permanent-view regression that flips the caller setting.
| val readBack = spark.read.format("avro").load(path) | ||
| assert(DataType.equalsIgnoreNullability(readBack.schema, input.schema)) | ||
| checkAnswer( | ||
| readBack.selectExpr("concat('<', c, '>')", "v", "concat('<', s.c, '>')"), |
There was a problem hiding this comment.
Non-blocking (P2): This case creates the array and map, but the projection only materializes c, v, and s.c. Column pruning can skip decoding both collection fields, so a regression in array/map conversion or CHAR-key padding would still pass. Please select a and m as well and assert Seq("q") and Map("k " -> "v").
2 participants
What changes were proposed in this pull request?
Parent: SPARK-58794 (first-class CHAR/VARCHAR under
spark.sql.charVarchar.standardSemantics.enabled).standardSemantics, keep Spark responsible for CHAR/VARCHAR assignment and scan checks on ORC by requesting physical ORC STRING. The row decoder accepts allStringTypesubtypes recursively. Preserve-only mode retains native ORC CHAR/VARCHAR enforcement.standardSemantics:standardSemantics=falsecollapses to STRING;preserveCharVarcharTypeInfokeeps typesParquet already embeds Spark schema JSON in footer metadata, so nested CHAR/VARCHAR inference needed tests rather than a writer change.
Why are the changes needed?
Without this, Spark-written files and user-specified schemas do not keep first-class CHAR/VARCHAR through round-trips in the remaining format gaps. For ORC specifically, an explicit
schema("c CHAR(4)")read sent native ORCchar(n)viaMAPRED_INPUT_SCHEMA, so ORC truncated"abcdef"to"abcd"beforecharTypeReadSideCheckandEXCEED_LIMIT_LENGTHnever fired.Does this PR introduce any user-facing change?
Yes, when
spark.sql.charVarchar.standardSemantics.enabledis true (unreleased / master):EXCEED_LIMIT_LENGTH.How was this patch tested?
New tests in
BasicCharVarcharTestSuite(SPARK-58814: major formats preserve CHAR/VARCHAR schemas and values) andAvroSuite(V1 and V2). Also ran existing ORC query suites and scalastyle:Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor Grok 4.6