srielau commented on code in PR #58545:
URL: https://github.com/apache/spark/pull/58545#discussion_r3942185031
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonParser.scala:
##########
@@ -617,13 +618,15 @@ class JacksonParser(
*/
private def convertMap(
parser: JsonParser,
- fieldConverter: ValueConverter): MapData = {
+ fieldConverter: ValueConverter,
+ keyType: DataType): MapData = {
val keys = ArrayBuffer.empty[UTF8String]
val values = ArrayBuffer.empty[Any]
var badRecordException: Option[Throwable] = None
while (nextUntil(parser, JsonToken.END_OBJECT)) {
- keys += UTF8String.fromString(parser.currentName)
+ keys += CharVarcharUtils.applyTextParseSemantics(
Review Comment:
CHAR padding can make distinct JSON keys equal. Here, `"a"` and `"a "` both
normalize to `"a "`:
```sql
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.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/StaxXmlParser.scala:
##########
@@ -374,11 +374,15 @@ class StaxXmlParser(
*/
private def convertMap(
parser: XMLEventReader,
+ keyType: DataType,
valueType: DataType,
attributes: Array[Attribute]): MapData = {
val kvPairs = ArrayBuffer.empty[(UTF8String, Any)]
+ def mapKey(raw: String): UTF8String = {
+ CharVarcharUtils.applyTextParseSemantics(UTF8String.fromString(raw),
keyType)
Review Comment:
Key normalization can introduce collisions that `kvPairs.toMap` silently
resolves. For example:
```sql
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.
--
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]
