Skip to content

Commit 35922ab

Browse files
srielauHyukjinKwon
authored andcommitted
[SPARK-60103][SQL] Length-check XML CHAR/VARCHAR map keys without pad or trim
### What changes were proposed in this pull request? When `spark.sql.charVarchar.standardSemantics.enabled` is true, `from_xml` and the XML datasource length-check XML names used as `MAP<CHAR(n), _>` / `MAP<VARCHAR(n), _>` keys without rewriting them. This matches SPARK-60108 for JSON. The checked names are element names, `attributePrefix` plus attribute names (default `_`), and the `valueTag` (default `_VALUE`) when mixed text is present. - `CHAR(n)` keys must already be exactly `n` characters (no pad). - `VARCHAR(n)` keys must already be at most `n` characters (no trim). - Exact repeated names last-win. `spark.sql.mapKeyDedupPolicy` is not applied. - Length failures follow the XML parse mode (`PERMISSIVE` / `FAILFAST`). Mismatched keys raise a new `UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY` condition (SQLSTATE `0A000`) instead of `EXCEED_LIMIT_LENGTH`, because pad/trim of map keys is not supported. XML CHAR/VARCHAR values still use the same pad and `EXCEED_LIMIT_LENGTH` checks as other parsed text. JSON, CSV, and TRANSFORM are unchanged. Struct-field maps still go through `convertField`. Empty, attribute-only, and text-only `MAP<STRING, _>` elements stay SQL NULL or a malformed record, as on master. `convertMap` is used for non-empty string-family keys (including CHAR/VARCHAR) and owns element bounding so a key-check failure still drains `ARRAY<MAP<...>>` siblings. This PR drops `convertConstrainedMap` and `DuplicateMapKeyUtils`. SPARK-59722-style assign-after-parse is not used. JIRA: https://issues.apache.org/jira/browse/SPARK-60103 ### Why are the changes needed? SPARK-59274 padded and trimmed CHAR/VARCHAR XML map keys during the walk, then applied `mapKeyDedupPolicy` to invented collisions. SPARK-59722 would have done the same via write-side assignment after a STRING parse; that approach was abandoned for JSON in favor of SPARK-60108. XML names are the key identity, so they should be length-checked in place rather than rewritten. ### Does this PR introduce _any_ user-facing change? Yes, under `spark.sql.charVarchar.standardSemantics.enabled=true`: ```sql SELECT from_xml('<ROW><m><a>1</a></m></ROW>', 'm MAP<CHAR(2), INT>', map('mode', 'FAILFAST')); -- [UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY] XML map keys of CHAR or VARCHAR cannot be padded or trimmed. -- The key 'a' is not valid for type "CHAR(2)". SQLSTATE: 0A000 ``` Exact-width CHAR keys and in-limit VARCHAR keys are kept as-is. Repeated names last-win. When the standard-semantics flag is false and `spark.sql.preserveCharVarcharTypeInfo` is true, short CHAR keys are padded and over-length CHAR or VARCHAR keys raise `EXCEED_LIMIT_LENGTH`. Documented in `docs/sql-migration-guide.md` (Spark SQL 4.3 to 4.4). In Spark 4.3, `from_xml` and XML reader schemas rejected CHAR/VARCHAR with `UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING`, or replaced them with STRING when `spark.sql.legacy.charVarcharAsString` was true. ### How was this patch tested? Added SPARK-60103 coverage in `BasicCharVarcharTestSuite` (too-short CHAR, exact-width CHAR, CHAR/VARCHAR overflow, last-wins duplicates, prefixed attributes and `valueTag` in mixed content, PERMISSIVE vs FAILFAST, nested maps, `ARRAY<MAP>`, collated keys, `from_xml` and the XML datasource, empty/attribute-only/text-only `MAP<STRING>`, `format("xml")` `MAP<INT>`, flag-off pad/overflow including over-length CHAR). Also re-ran SPARK-59274 XML value, sibling, and `rowTag` tests. Ran: ``` JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 build/sbt -Dsbt.override.build.repos=true \ 'sql/testOnly org.apache.spark.sql.BasicCharVarcharTestSuite -- -z SPARK-60103 -z "SPARK-59274: from_json/csv/xml" -z "SPARK-59274: ordinary STRING" -z "SPARK-59274: collated" -z "SPARK-59274: XML rowTag" -z "SPARK-59274: mixed XML" -z "SPARK-59274: nested XML"' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Cursor Grok 4.6 Closes #59316 from srielau/SPARK-60103. Authored-by: Serge Rielau <serge@rielau.com> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit ccc8f69) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
1 parent b628a94 commit 35922ab

7 files changed

Lines changed: 376 additions & 345 deletions

File tree

‎common/utils/src/main/resources/error/error-conditions.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9889,6 +9889,13 @@
98899889
],
98909890
"sqlState" : "0A000"
98919891
},
9892+
"UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY" : {
9893+
"message" : [
9894+
"XML map keys of CHAR or VARCHAR cannot be padded or trimmed.",
9895+
"The key <key> is not valid for type <dataType>."
9896+
],
9897+
"sqlState" : "0A000"
9898+
},
98929899
"UNTYPED_SCALA_UDF" : {
98939900
"message" : [
98949901
"You're using untyped Scala UDF, which does not have the input type information. Spark may blindly pass null to the Scala closure with primitive-type argument, and the closure will see the default value of the Java type for the null argument, e.g. `udf((x: Int) => x, IntegerType)`, the result is 0 for null input. To get rid of this error, you could:",

‎docs/sql-migration-guide.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ license: |
2525
## Upgrading from Spark SQL 4.3 to 4.4
2626

2727
- Since Spark 4.4, when `spark.sql.preserveCharVarcharTypeInfo` is true and `spark.sql.charVarchar.standardSemantics.enabled` is false, ORC reads that apply a CHAR/VARCHAR schema over STRING storage return the stored values without ORC truncation, matching Parquet. Previously the ORC reader requested `char(n)`/`varchar(n)` and truncated STRING-stored values to `n`. Read-side length checks (`EXCEED_LIMIT_LENGTH`) apply only when `spark.sql.charVarchar.standardSemantics.enabled` is true.
28+
- Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is true, XML names used as `MAP<CHAR(n), _>` or `MAP<VARCHAR(n), _>` keys in `from_xml` and the XML datasource are length-checked without padding or trimming. The checked names are element names, `attributePrefix` plus attribute names (default prefix `_`), and the `valueTag` (default `_VALUE`) when mixed text is present. A `CHAR(n)` key must already be exactly `n` characters, and a `VARCHAR(n)` key must already be at most `n` characters. Mismatched keys fail with `UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY` (SQLSTATE `0A000`) and follow the XML parse mode (`PERMISSIVE` or `FAILFAST`). Exact repeated names last-win; `spark.sql.mapKeyDedupPolicy` is not applied. XML CHAR/VARCHAR values use the same pad and `EXCEED_LIMIT_LENGTH` checks as other parsed text. Empty, attribute-only, and text-only map elements stay SQL NULL or a malformed record; they do not become an empty map or a `valueTag` entry. In Spark 4.3, `from_xml` and XML reader schemas rejected CHAR/VARCHAR with `UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING`, or replaced them with STRING when `spark.sql.legacy.charVarcharAsString` was true, so CHAR/VARCHAR map keys did not reach the parser. When the standard-semantics flag is false and `spark.sql.preserveCharVarcharTypeInfo` is true, short CHAR keys are padded and over-length CHAR or VARCHAR keys raise `EXCEED_LIMIT_LENGTH`.
2829
- Since Spark 4.4, the options maps passed to `from_csv`, `to_csv`, `schema_of_csv`, `from_json`, `to_json`, `schema_of_json`, `from_xml`, `to_xml`, and `schema_of_xml` must be foldable after replacing `RuntimeReplaceable` expressions. Previously, Spark evaluated non-foldable options during analysis, which allowed some constant expressions but could fail with an internal error or incorrectly evaluate row-dependent expressions. To allow deterministic and row-independent non-foldable options, set `spark.sql.legacy.allowNonFoldableOptions` to `true`. Row-dependent, unevaluable, and nondeterministic options are always rejected.
2930
- Since Spark 4.4, when an already-analyzed Data Source V2 query is refreshed after a compatible schema change, connectors can return more data columns from the current table schema in `Scan.readSchema()` than requested by `SupportsPushDownRequiredColumns.pruneColumns`. Previously, this partial pruning could fail planning because the scan reported columns absent from the analyzed relation output.
3031
- Since Spark 4.4, when an already-analyzed Data Source V2 query is refreshed, if rebinding an expanded struct used as a map key causes distinct current keys to collide under the analyzed schema, the query fails with `DUPLICATED_MAP_KEY` by default instead of returning duplicate keys. With `spark.sql.mapKeyDedupPolicy=LAST_WIN`, the last value is kept.

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DuplicateMapKeyUtils.scala‎

Lines changed: 0 additions & 82 deletions
This file was deleted.

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/StaxXmlParser.scala‎

Lines changed: 73 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import javax.xml.stream.events._
2626
import javax.xml.transform.stream.StreamSource
2727
import javax.xml.validation.Schema
2828

29-
import scala.collection.mutable
3029
import scala.collection.mutable.ArrayBuffer
3130
import scala.jdk.CollectionConverters._
3231
import scala.util.Try
@@ -42,7 +41,7 @@ import org.apache.spark.{SparkIllegalArgumentException, SparkUpgradeException}
4241
import org.apache.spark.internal.Logging
4342
import org.apache.spark.sql.catalyst.InternalRow
4443
import org.apache.spark.sql.catalyst.expressions.{ExprUtils, GenericInternalRow, ToStringBase}
45-
import org.apache.spark.sql.catalyst.util.{ArrayBasedMapData, BadRecordException, CharVarcharUtils, DateFormatter, DropMalformedMode, DuplicateMapKeyUtils, FailureSafeParser, GenericArrayData, MapData, ParseMode, PartialResultArrayException, PartialResultException, PermissiveMode, TimeFormatter, TimestampFormatter}
44+
import org.apache.spark.sql.catalyst.util.{ArrayBasedMapData, BadRecordException, CharVarcharUtils, DateFormatter, DropMalformedMode, FailureSafeParser, GenericArrayData, MapData, ParseMode, PartialResultArrayException, PartialResultException, PermissiveMode, TimeFormatter, TimestampFormatter}
4645
import org.apache.spark.sql.catalyst.util.LegacyDateFormats.FAST_DATE_FORMAT
4746
import org.apache.spark.sql.catalyst.xml.StaxXmlParser.convertStream
4847
import org.apache.spark.sql.errors.QueryExecutionErrors
@@ -86,6 +85,9 @@ class StaxXmlParser(
8685

8786
private val caseSensitive = SQLConf.get.caseSensitiveAnalysis
8887

88+
// CHAR/VARCHAR XML map keys are length-checked without pad or trim under this flag.
89+
private val charVarcharStandardSemantics = SQLConf.get.charVarcharStandardSemantics
90+
8991
/**
9092
* Limits a view of an event stream to one element whose start event has already been consumed.
9193
* Closing or draining this view consumes the matching end event without closing the underlying
@@ -233,7 +235,6 @@ class StaxXmlParser(
233235
// ValidatorUtil.newValidator throws this when the JAXP implementation cannot
234236
// disable external access; that is an environment error, not a bad record.
235237
case e: UnsupportedOperationException => throw e
236-
case DuplicateMapKeyUtils(e) => throw e
237238
case e@(_: RuntimeException | _: XMLStreamException | _: MalformedInputException
238239
| _: SAXException) =>
239240
// XML parser currently doesn't support partial results for corrupted records.
@@ -330,16 +331,11 @@ class StaxXmlParser(
330331
throw BadRecordException(xmlLiteral, () => Array.empty,
331332
wrappedCharException)
332333
case PartialResultException(row, cause) =>
333-
DuplicateMapKeyUtils.cause(cause) match {
334-
case Some(e) => throw e
335-
case None =>
336-
throw BadRecordException(record = xmlLiteral, partialResults = () => Array(row), cause)
337-
}
334+
throw BadRecordException(record = xmlLiteral, partialResults = () => Array(row), cause)
338335
case PartialResultArrayException(rows, cause) =>
339336
throw BadRecordException(record = xmlLiteral, partialResults = () => rows, cause)
340337
case e: Throwable =>
341338
SparkErrorUtils.getRootCause(e) match {
342-
case DuplicateMapKeyUtils(duplicate) => throw duplicate
343339
case _: FileNotFoundException if options.ignoreMissingFiles =>
344340
logWarning("Skipped missing file", e)
345341
parser.close()
@@ -383,9 +379,10 @@ class StaxXmlParser(
383379
startElementName: String,
384380
attributes: Array[Attribute]): Any = dt match {
385381
case st: StructType => convertObject(parser, st)
386-
case MapType(StringType, vt, _) => convertMap(parser, vt, attributes)
387-
case MapType(kt @ (_: CharType | _: VarcharType), vt, _) =>
388-
convertConstrainedMap(parser, kt, vt, attributes)
382+
// CHAR/VARCHAR extend StringType. Non-string keys (for example MAP<INT, _> through
383+
// format("xml").load()) must not reach convertMap: applyTextParseSemantics would keep
384+
// UTF8String keys and fail later with ClassCastException.
385+
case MapType(kt: StringType, vt, _) => convertMap(parser, vt, attributes, kt)
389386
case ArrayType(st, _) => convertField(parser, st, startElementName)
390387
case VariantType =>
391388
StaxXmlParser.convertVariant(parser, attributes, options)
@@ -445,109 +442,80 @@ class StaxXmlParser(
445442
}
446443

447444
/**
448-
* Parse an object as map.
445+
* Parse an object as a Map.
446+
*
447+
* When `spark.sql.charVarchar.standardSemantics.enabled` is true, XML names used as
448+
* CHAR/VARCHAR keys are length-checked without rewriting: CHAR keys must already be
449+
* exactly n characters, and VARCHAR keys must already be at most n characters.
450+
* Padding, trimming, and mapKeyDedupPolicy are not applied. With the flag off,
451+
* short CHAR keys are padded and over-length CHAR or VARCHAR keys raise
452+
* EXCEED_LIMIT_LENGTH when first-class CHAR/VARCHAR types reach the parser.
453+
* Repeated names last-win on binary equality (collation is not consulted), matching
454+
* ordinary MAP<STRING, ...> XML maps.
455+
*
456+
* Example (flag on): `from_xml('<ROW><m><ab>1</ab></m></ROW>',
457+
* 'm MAP<CHAR(2), INT>')` keeps key `ab`; `MAP<CHAR(4), INT>` raises
458+
* `UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY`.
459+
*
460+
* This method owns element bounding so a key-check failure still consumes the current
461+
* map element (ARRAY<MAP<...>> and nested maps included).
449462
*/
450463
private def convertMap(
451464
parser: XMLEventReader,
452465
valueType: DataType,
453-
attributes: Array[Attribute]): MapData = {
454-
val kvPairs = ArrayBuffer.empty[(UTF8String, Any)]
455-
attributes.foreach { attr =>
456-
kvPairs += (UTF8String.fromString(options.attributePrefix + attr.getName.getLocalPart)
457-
-> convertTo(attr.getValue, valueType))
458-
}
459-
var shouldStop = false
460-
while (!shouldStop) {
461-
parser.nextEvent match {
462-
case e: StartElement =>
463-
val key = StaxXmlParserUtils.getName(e.asStartElement.getName, options)
464-
kvPairs +=
465-
(UTF8String.fromString(key) -> convertField(parser, valueType, key))
466-
case c: Characters if !c.isWhiteSpace =>
467-
// Create a value tag field for it
468-
kvPairs +=
469-
// TODO: We don't support array value tags in maps yet.
470-
(UTF8String.fromString(options.valueTag) -> convertTo(c.getData, valueType))
471-
case _: EndElement | _: EndDocument =>
472-
shouldStop = true
473-
case _ => // do nothing
466+
attributes: Array[Attribute],
467+
keyType: DataType): MapData = {
468+
val bounded = new ElementBoundedEventReader(parser)
469+
try {
470+
val kvPairs = ArrayBuffer.empty[(UTF8String, Any)]
471+
attributes.foreach { attr =>
472+
val key = convertXmlMapKey(
473+
options.attributePrefix + attr.getName.getLocalPart, keyType)
474+
kvPairs += (key -> convertTo(attr.getValue, valueType))
475+
}
476+
var shouldStop = false
477+
while (!shouldStop) {
478+
bounded.nextEvent match {
479+
case e: StartElement =>
480+
val rawName = StaxXmlParserUtils.getName(e.asStartElement.getName, options)
481+
val key = convertXmlMapKey(rawName, keyType)
482+
kvPairs += (key -> convertField(bounded, valueType, rawName))
483+
case c: Characters if !c.isWhiteSpace =>
484+
// Create a value tag field for it
485+
kvPairs +=
486+
// TODO: We don't support array value tags in maps yet.
487+
(convertXmlMapKey(options.valueTag, keyType) -> convertTo(c.getData, valueType))
488+
case _: EndElement | _: EndDocument =>
489+
shouldStop = true
490+
case _ => // do nothing
491+
}
474492
}
493+
ArrayBasedMapData(kvPairs.toMap)
494+
} finally {
495+
bounded.drain()
475496
}
476-
ArrayBasedMapData(kvPairs.toMap)
477497
}
478498

479-
private def convertConstrainedMap(
480-
parser: XMLEventReader,
481-
keyType: DataType,
482-
valueType: DataType,
483-
attributes: Array[Attribute]): MapData = {
484-
val lastEntries =
485-
mutable.LinkedHashMap.empty[UTF8String, (UTF8String, Option[Any])]
486-
var badMapException: Option[Throwable] = None
487-
def mapKey(raw: UTF8String): UTF8String = {
488-
CharVarcharUtils.applyTextParseSemantics(raw, keyType)
489-
}
490-
def appendPair(rawKey: String, value: Option[Any]): Unit = {
491-
try {
492-
val rawKeyUtf8 = UTF8String.fromString(rawKey)
493-
lastEntries.remove(rawKeyUtf8)
494-
lastEntries.update(rawKeyUtf8, (mapKey(rawKeyUtf8), value))
495-
} catch {
496-
case NonFatal(e) => badMapException = badMapException.orElse(Some(e))
497-
}
498-
}
499-
attributes.foreach { attr =>
500-
val value = try {
501-
Some(convertTo(attr.getValue, valueType))
502-
} catch {
503-
case e: SparkUpgradeException => throw e
504-
case NonFatal(e) =>
505-
badMapException = badMapException.orElse(Some(e))
506-
None
507-
}
508-
appendPair(options.attributePrefix + attr.getName.getLocalPart, value)
509-
}
510-
var shouldStop = false
511-
while (!shouldStop) {
512-
parser.nextEvent match {
513-
case e: StartElement =>
514-
val rawKey = StaxXmlParserUtils.getName(e.asStartElement.getName, options)
515-
val entryParser = new ElementBoundedEventReader(parser)
516-
val value = {
517-
try {
518-
Some(convertField(entryParser, valueType, rawKey))
519-
} catch {
520-
case e: SparkUpgradeException => throw e
521-
case DuplicateMapKeyUtils(e) => throw e
522-
case NonFatal(e) =>
523-
badMapException = badMapException.orElse(Some(e))
524-
None
525-
} finally {
526-
entryParser.drain()
527-
}
528-
}
529-
appendPair(rawKey, value)
530-
case c: Characters if !c.isWhiteSpace =>
531-
// Create a value tag field for it
532-
// TODO: We don't support array value tags in maps yet.
533-
val value = try {
534-
Some(convertTo(c.getData, valueType))
535-
} catch {
536-
case e: SparkUpgradeException => throw e
537-
case NonFatal(e) =>
538-
badMapException = badMapException.orElse(Some(e))
539-
None
540-
}
541-
appendPair(options.valueTag, value)
542-
case _: EndElement | _: EndDocument =>
543-
shouldStop = true
544-
case _ => // do nothing
499+
/**
500+
* Length-check a CHAR/VARCHAR XML name used as a map key. Unlike value assignment,
501+
* this does not pad or trim. STRING keys and the flag-off path still use
502+
* [[CharVarcharUtils.applyTextParseSemantics]].
503+
*/
504+
private def convertXmlMapKey(rawName: String, keyType: DataType): UTF8String = {
505+
val key = UTF8String.fromString(rawName)
506+
if (charVarcharStandardSemantics) {
507+
keyType match {
508+
case c: CharType if key.numChars() != c.length =>
509+
throw QueryExecutionErrors.unsupportedXmlCharVarcharMapKey(key, c)
510+
case _: CharType => key
511+
case v: VarcharType if key.numChars() > v.length =>
512+
throw QueryExecutionErrors.unsupportedXmlCharVarcharMapKey(key, v)
513+
case _: VarcharType => key
514+
case _ => CharVarcharUtils.applyTextParseSemantics(key, keyType)
545515
}
516+
} else {
517+
CharVarcharUtils.applyTextParseSemantics(key, keyType)
546518
}
547-
val mapData = DuplicateMapKeyUtils.buildConstrainedMap(
548-
lastEntries, keyType, valueType)
549-
badMapException.foreach(throw _)
550-
mapData
551519
}
552520

553521
/**
@@ -583,7 +551,6 @@ class StaxXmlParser(
583551
row(i) = convertTo(v, schema(i).dataType)
584552
} catch {
585553
case e: SparkUpgradeException => throw e
586-
case DuplicateMapKeyUtils(e) => throw e
587554
case NonFatal(e) => firstError = firstError.orElse(Some(e))
588555
}
589556
}
@@ -722,7 +689,6 @@ class StaxXmlParser(
722689
}
723690
} catch {
724691
case e: SparkUpgradeException => throw e
725-
case DuplicateMapKeyUtils(e) => throw e
726692
case NonFatal(e) =>
727693
// TODO: we don't support partial results now
728694
badRecordException = badRecordException.orElse(Some(e))

‎sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,6 +2727,20 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
27272727
)
27282728
}
27292729

2730+
/**
2731+
* Restricting CHAR/VARCHAR XML map keys is a parser limitation (SQLSTATE 0A000),
2732+
* but this is a SparkRuntimeException so XML parse modes can handle it: PERMISSIVE
2733+
* wraps a bad record and FAILFAST surfaces the failure.
2734+
*/
2735+
def unsupportedXmlCharVarcharMapKey(
2736+
key: UTF8String, dataType: DataType): SparkRuntimeException = {
2737+
new SparkRuntimeException(
2738+
errorClass = "UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY",
2739+
messageParameters = Map(
2740+
"key" -> toSQLValue(key, StringType),
2741+
"dataType" -> toSQLType(dataType)))
2742+
}
2743+
27302744
def timestampAddOverflowError(micros: Long, amount: Long, unit: String): ArithmeticException = {
27312745
new SparkArithmeticException(
27322746
errorClass = "DATETIME_OVERFLOW",

0 commit comments

Comments
 (0)