[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>')

[srielau]

Key normalization can introduce collisions that kvPairs.toMap silently resolves. For example:

SELECT from_xml(
  '<ROW><m>9<a>1</a></m></ROW>',
  'm MAP<CHAR(2), INT>',
  map('valueTag', 'a ')).m;

The value-tag key "a " and element key "a" both become "a ", causing silent data loss. Please build the map with duplicate-key handling so EXCEPTION reports DUPLICATED_MAP_KEY and LAST_WIN behaves consistently. Add coverage for both policies.

[srielau]

Fixed. XML convertMap now uses ArrayBasedMapBuilder instead of kvPairs.toMap, so CHAR-normalized collisions honor spark.sql.mapKeyDedupPolicy rather than silently dropping a key.

Regression (valueTag 'a ' collides with padded <a>):

SELECT from_xml(
  '<ROW><m><a>1</a>9</m></ROW>',
  'm MAP<CHAR(2), INT>',
  map('valueTag', 'a ')).m

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

Blocking (P1): ArrayBasedMapBuilder is now used for every StringType-keyed map, not only CHAR/VARCHAR keys. That changes existing default-mode behavior: duplicate JSON object names (and repeated XML map element names) now raise DUPLICATED_MAP_KEY, while the old JSON path constructed MapData directly and the old XML path used last-wins toMap. This is reachable with the standard-semantics flag still off, e.g. from_json('{"a":1,"a":2}', 'MAP<STRING, INT>'). Could we preserve the old construction path for ordinary STRING keys and select policy-aware construction only where CHAR/VARCHAR normalization can introduce collisions?

Recommended change: Keep the previous JSON/XML map construction behavior for every non-CHAR/VARCHAR string key type, and use ArrayBasedMapBuilder plus duplicate-key escape handling only for CHAR/VARCHAR keys whose new normalization can create collisions.

Why this works: Classify the declared map key type at the parser conversion boundary instead of treating every StringType subtype as part of the new deduplication path; gate the corresponding duplicate-error bypass by the same classification.

Scope: JacksonParser.convertMap, StaxXmlParser.convertMap, their duplicate-error propagation hooks, and compatibility tests for ordinary STRING maps alongside the CHAR/VARCHAR collision tests.

Compatibility: Ordinary MAP<STRING, ...> inputs retain their pre-PR duplicate behavior with standard semantics both disabled and enabled, while normalized CHAR/VARCHAR collisions continue to honor EXCEPTION and LAST_WIN.

Risks: The type gate must distinguish first-class CHAR/VARCHAR keys without changing behavior for other StringType variants or collations. The JSON and XML construction branches must remain aligned with their respective pre-PR duplicate semantics.

Constraints: Do not broaden spark.sql.mapKeyDedupPolicy beyond the normalized CHAR/VARCHAR collision path introduced by this PR. Preserve existing parse-mode and partial-result behavior for ordinary STRING maps.

Success: The ordinary JSON and XML duplicate examples return their pre-PR last value under default settings, and CHAR/VARCHAR normalization collisions still throw under EXCEPTION and retain the last value under LAST_WIN.

[cloud-fan]

Blocking (P1): The new top-level bypass only works while DUPLICATED_MAP_KEY remains the root cause. Here an outer convertMap can catch an inner duplicate as generic NonFatal, omit its value, and then replace it with the key/value-length error from from; convertObject can likewise retain an earlier field error and discard a later duplicate. In both cases PERMISSIVE mode can bypass mapKeyDedupPolicy=EXCEPTION. Could we rethrow or prioritize this condition in the inner partial-result paths and cover both a map nested as a map value and a duplicate after an unrelated bad struct field?

Recommended change: Make DUPLICATED_MAP_KEY terminal at every JacksonParser partial-result boundary while leaving other conversion failures under the existing permissive partial-result policy.

Why this works: Detect the duplicate-key condition before generic NonFatal handling in convertMap and before first-error accumulation can hide it in convertObject, using one consistent condition/cause check across the parser.

Scope: JacksonParser convertMap and convertObject error handling plus BasicCharVarcharTestSuite regressions for nested-map and earlier-bad-field ordering cases.

Compatibility: EXCEPTION reliably fails on normalized duplicates regardless of nesting or field order; LAST_WIN and partial results for non-duplicate conversion errors remain unchanged.

Risks: An overly broad cause-chain rethrow could disable permissive handling for unrelated SparkRuntimeException conditions. Changing accumulator precedence must not discard valid partial results for errors other than duplicate map keys.

Constraints: Match only the established DUPLICATED_MAP_KEY condition. Preserve JSON partial-result behavior and parse-mode handling for all other failures.

Success: A CHAR-key collision nested under another map and the same collision after an unrelated malformed struct field both raise DUPLICATED_MAP_KEY under EXCEPTION, without changing non-duplicate permissive results.