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]

Reply via email to