[SPARK-59274][SQL] Enforce CHAR/VARCHAR semantics in schema-driven text parsing

[srielau]

What changes were proposed in this pull request?

When spark.sql.charVarchar.standardSemantics.enabled is true, first-class CHAR/VARCHAR schemas are already allowed through failIfHasCharVarchar. Schema-driven text parsers still treated those columns as unbounded STRING (no pad/overflow), schema_of_json/csv/xml rejected CHAR/VARCHAR input with child.dataType != StringType, and XML map keys / convertTo used exact StringType matches.

This patch applies assignment semantics while parsing text into a typed schema:

• JSON (JacksonParser), CSV (UnivocityParser), and XML (StaxXmlParser) call CharVarcharUtils.applyTextParseSemantics so CHAR is padded and oversize VARCHAR raises EXCEED_LIMIT_LENGTH
• JSON/XML map keys with CHAR/VARCHAR key types get the same checks
• XML wildcard columns and convertTo keep the declared CHAR/VARCHAR type instead of collapsing to StringType
• schema_of_json / schema_of_csv / schema_of_xml accept any StringType subtype as the input document

Default (flag off) is unchanged: from_json(..., 'a CHAR(5)') still fails with UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING.

Why are the changes needed?

from_json / from_csv / from_xml are the remaining schema-driven text parse surfaces that drop CHAR/VARCHAR length rules under standard semantics. Without this, enabling the flag still produces unpadded CHAR values, silently truncates or accepts oversize VARCHAR, and rejects CHAR/VARCHAR documents in schema_of_*.

JIRA: https://issues.apache.org/jira/browse/SPARK-59274 (subtask of SPARK-58794)

Does this PR introduce any user-facing change?

Yes, when spark.sql.charVarchar.standardSemantics.enabled is true (still default false):

• from_json / from_csv / from_xml with a CHAR/VARCHAR schema keep those types and apply assignment checks (CHAR pad, VARCHAR overflow -> EXCEED_LIMIT_LENGTH).
• schema_of_json / schema_of_csv / schema_of_xml accept CHAR/VARCHAR input strings.
• JSON/XML maps with CHAR/VARCHAR keys pad or reject keys the same way.

How was this patch tested?

Added BasicCharVarcharTestSuite coverage for SPARK-59274: CHAR padding and VARCHAR overflow in from_json / from_csv / from_xml, JSON/XML MAP<CHAR(n), INT> keys, and schema_of_json/csv/xml on VARCHAR input.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor Grok 4.6

[srielau]

CHAR padding can make distinct JSON keys equal. Here, "a" and "a " both normalize to "a ":

SELECT map_keys(from_json(
  '{"a":1,"a ":2}',
  'MAP<CHAR(2), INT>'));

This path then constructs ArrayBasedMapData directly (the "JSON map will never have duplicated keys" comment below is no longer true), returning duplicate physical keys. Please detect normalized-key collisions, preferably through ArrayBasedMapBuilder, so they honor spark.sql.mapKeyDedupPolicy. Please also add this regression test.

[srielau]

Fixed. convertMap now builds through ArrayBasedMapBuilder so CHAR-normalized keys go through spark.sql.mapKeyDedupPolicy instead of constructing ArrayBasedMapData with assumed-unique keys.

DUPLICATED_MAP_KEY is rethrown from the JSON parser and FailureSafeParser so EXCEPTION vs LAST_WIN is independent of parse mode.

Regression:

SELECT from_json('{"a":1,"a ":2}', 'MAP<CHAR(2), INT>')

[cloud-fan]

Review summary

The CHAR/VARCHAR parsing direction is sound, but the current builder integration has two blocking correctness regressions. First, it changes ordinary JSON/XML MAP<STRING, ...> duplicate handling under default settings, outside the feature flag's intended scope. Second, JSON partial-result handling can intercept or overwrite normalized duplicate-key errors, so mapKeyDedupPolicy=EXCEPTION varies with nesting and field order. Both need correction before merge.

No tests were run as part of this review; the repository instructions require checking whether the user has more changes before voluntary test execution.

Findings

2 total: 0 P0, 2 P1, 0 P2, 0 P3.

Blocking (P1)

• Preserve duplicate handling for ordinary STRING maps — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonParser.scala:643 — see inline.
• Preserve duplicate-key errors through nested JSON parsing — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonParser.scala:643 — see inline.

[cloud-fan]

Review summary

The shared scalar normalization helper and schema-expression changes are reasonable, and CI is currently green. However, the JSON constrained-map path commits keys before consuming values, breaking partial-result parser alignment and duplicate-policy error handling; this P1 must be fixed before merge. The JSON/XML map construction also needs to preserve exact raw-duplicate compatibility without losing normalization-created collision handling, and the regression matrix should add over-limit CHAR coverage for JSON, CSV, and XML.

Findings

3 total: 0 P0, 1 P1, 2 P2, 0 P3.

Blocking (P1)

• Consume a JSON map value before committing its normalized key — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonParser.scala:640 — see inline.

Non-blocking (P2)

• Preserve exact duplicate names while detecting normalization collisions — General
  The new policy-aware builder also changes exact-duplicate compatibility for both parsers. With standard semantics and the default EXCEPTION policy, an input such as {'a':1,'a':2} parsed as MAP<VARCHAR(2), INT> now throws, although the same raw-name duplicate was historically accepted and normalization introduced no collision. Please preserve the parser's historical result for exact repeated raw names while still applying mapKeyDedupPolicy when distinct raw names normalize together. Tests should include CHAR padding collisions and the VARCHAR-specific a versus a collision for VARCHAR(1), under both EXCEPTION and LAST_WIN, in JSON and XML.

  Recommended change: Distinguish exact repeated serialized names from collisions introduced by CHAR/VARCHAR normalization, preserving legacy exact-duplicate behavior while applying mapKeyDedupPolicy to the latter.

  Why this works: Retain each raw key alongside its normalized key during JSON and XML map assembly. Resolve exact raw-name repeats with the parser's historical behavior, but send distinct raw names that share a normalized key through the configured EXCEPTION or LAST_WIN policy.

  Scope: The constrained-key map construction branches in JacksonParser and StaxXmlParser, with JSON and XML regression coverage for exact repeats and normalization-created collisions for both CHAR and VARCHAR.

  Compatibility: Exact repeated raw names remain accepted as before; collisions created by CHAR padding or VARCHAR trailing-space trimming continue to honor mapKeyDedupPolicy. Ordinary STRING keys and behavior with standard semantics disabled remain unchanged.

  Risks: Collapsing raw duplicates too early can hide a later collision from a distinct raw key. JSON and XML had different legacy assembly paths, so a shared strategy must preserve each parser's observable result.

  Constraints: Do not bypass policy handling for a and a parsed as VARCHAR(1). Preserve LAST_WIN ordering after normalization. Keep the ordinary STRING-key compatibility path unchanged.

  Success: Exact repeated CHAR/VARCHAR raw keys retain their pre-change observable result, while distinct raw keys that normalize together raise DUPLICATED_MAP_KEY under EXCEPTION and retain the last normalized value under LAST_WIN in both JSON and XML.

• Exercise non-space CHAR overflow on every text-parser path — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala:171 — see inline.

Re-review status

Prior AI findings: 2 addressed, 0 still present; additional unresolved findings in this review: 3.

New attribution: 0 newly introduced, 3 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

Existing discussions

• existing discussion — The simple JSON normalization collision is covered, but a malformed earlier value still prevents the same-map collision from being governed by mapKeyDedupPolicy.
• existing discussion — The explicit extractor fixes nested duplicates and duplicates after an earlier bad struct field, but a malformed value in the same map still changes the arrays before duplicate policy is evaluated.

[cloud-fan]

Review summary

The shared normalization and parser coverage are generally coherent, and the latest revision addresses the earlier JSON recovery and duplicate-compatibility concerns. One blocking XML recovery defect remains: an over-limit constrained map key is normalized before its element is consumed, allowing permissive recovery to terminate the enclosing row early and discard valid sibling fields. Please also add focused coverage for the new empty-element CHAR branch.

No tests were run as part of this review; the repository instructions require checking whether the user has more changes before voluntary test execution.

Findings

2 total: 0 P0, 1 P1, 1 P2, 0 P3.

Blocking (P1)

• Consume an XML map entry before surfacing a constrained-key error — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/StaxXmlParser.scala:397 — see inline.

Non-blocking (P2)

• Cover CHAR semantics for empty XML elements — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/StaxXmlParser.scala:337 — see inline.

Re-review status

Prior AI findings: 3 addressed, 0 still present; additional unresolved findings in this review: 2.

New attribution: 0 newly introduced, 2 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

Verification

• The blocking recovery path is reached through public from_xml evaluation with standard CHAR/VARCHAR semantics enabled and default permissive parsing.

[cloud-fan]

Blocking (P1): mapKey(key) can throw before convertField consumes <abc>. In permissive mode, the enclosing convertObject then resumes on the same reader and can treat </abc> as the end of the row, so from_xml('<ROW><m><abc>1</abc></m><tail>2</tail></ROW>', 'm MAP<CHAR(2), INT>, tail INT') loses tail = 2. Please consume or recover the entry value before applying constrained-key semantics, and append the key/value pair atomically so a rejected key leaves the reader positioned at the next sibling.

Recommended change: Consume or recover each XML map entry value before applying CHAR/VARCHAR key normalization, then commit the normalized key and converted value together.

Why this works: Moving key validation after value consumption preserves the XMLEventReader position expected by convertObject's recoverable-error path and prevents partially committed map entries.

Scope: StaxXmlParser.convertMap and focused XML partial-result tests for an oversized constrained map key followed by a valid sibling field.

Compatibility: Successful maps, ordinary STRING-key handling, parse modes, and duplicate-policy behavior should remain unchanged; only recovery after an invalid constrained key is corrected.

Risks: Changing conversion order can alter which error wins when both a key and its value are invalid. Incorrect event advancement could skip a nested value or the following map entry.

Constraints: Invoke the value converter at most once per entry. Keep key and value assembly atomic on every partial-result path. Preserve the configured normalized-key duplicate policy.

Success: With partial results enabled, an oversized CHAR/VARCHAR XML map key no longer hides a valid later row field, while existing successful and duplicate-policy cases retain their behavior.

[cloud-fan]

Non-blocking (P2): Please add a focused BasicCharVarcharTestSuite case for from_xml('<ROW><a></a></ROW>', 'a CHAR(5)', map('nullValue', 'NULL')) and assert exactly five spaces. The existing character-content and overflow cases do not exercise this empty-element branch or distinguish the required padded value from null and "".