cloud-fan commented on code in PR #58545:
URL: https://github.com/apache/spark/pull/58545#discussion_r4008384565
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/StaxXmlParser.scala:
##########
@@ -374,31 +378,54 @@ class StaxXmlParser(
*/
private def convertMap(
parser: XMLEventReader,
+ keyType: DataType,
valueType: DataType,
attributes: Array[Attribute]): MapData = {
val kvPairs = ArrayBuffer.empty[(UTF8String, Any)]
+ var mapKeyException: Option[Throwable] = None
+ def mapKey(raw: String): UTF8String = {
+ CharVarcharUtils.applyTextParseSemantics(UTF8String.fromString(raw),
keyType)
+ }
+ def appendPair(rawKey: String, value: Any): Unit = {
+ try {
+ kvPairs += (mapKey(rawKey) -> value)
+ } catch {
+ case NonFatal(e) => mapKeyException = mapKeyException.orElse(Some(e))
+ }
+ }
attributes.foreach { attr =>
- kvPairs += (UTF8String.fromString(options.attributePrefix +
attr.getName.getLocalPart)
- -> convertTo(attr.getValue, valueType))
+ val value = convertTo(attr.getValue, valueType)
+ appendPair(options.attributePrefix + attr.getName.getLocalPart, value)
}
var shouldStop = false
while (!shouldStop) {
parser.nextEvent match {
case e: StartElement =>
- val key = StaxXmlParserUtils.getName(e.asStartElement.getName,
options)
- kvPairs +=
- (UTF8String.fromString(key) -> convertField(parser, valueType, key))
+ val rawKey = StaxXmlParserUtils.getName(e.asStartElement.getName,
options)
+ val value = convertField(parser, valueType, rawKey)
+ appendPair(rawKey, value)
case c: Characters if !c.isWhiteSpace =>
// Create a value tag field for it
- kvPairs +=
// TODO: We don't support an array value tags in map yet.
- (UTF8String.fromString(options.valueTag) -> convertTo(c.getData,
valueType))
+ val value = convertTo(c.getData, valueType)
+ appendPair(options.valueTag, value)
case _: EndElement | _: EndDocument =>
shouldStop = true
case _ => // do nothing
}
}
- ArrayBasedMapData(kvPairs.toMap)
+ keyType match {
+ case _: CharType | _: VarcharType =>
+ val mapBuilder = new ArrayBasedMapBuilder(keyType, valueType)
+ kvPairs.foreach { case (key, value) => mapBuilder.put(key, value) }
+ val mapData = mapBuilder.build()
+ mapKeyException.foreach(throw _)
+ mapData
+ case _ =>
Review Comment:
**Blocking (P1):** This fallback now includes non-binary collated StringType
keys because the caller accepts any StringType. For a UTF8_LCASE map, keys such
as `a` and `A` are equal under the declared type, but `kvPairs.toMap` compares
their UTF8String bytes and retains both, so neither `EXCEPTION` nor `LAST_WIN`
is applied. Please route non-binary collated keys through
`ArrayBasedMapBuilder`, keep this historical path only for ordinary UTF8_BINARY
STRING keys, and add coverage for both policy outcomes.
--
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]