Preserve Legacy Behavior for Duplicate Fields in Nested Rows

This change makes it so that nested rows keep only the first of
duplicate fields. This is different from top-level struct fields
and maps, which both keep the last value.

I'd like to have a SerDe property to manage this behavior but don't
want to add any more complexity at this point.

Author: rmarrowstone

Diff for: lib/trino-hive-formats/src/main/java/io/trino/hive/formats/ion/IonDecoderFactory.java

@@ -235,13 +235,10 @@ private void decode(IonReader ionReader, IntFunction<BlockBuilder> blockSelector
                     continue;
                 }
                 final BlockBuilder blockBuilder = blockSelector.apply(fieldIndex);
-                if (encountered[fieldIndex]) {
-                    blockBuilder.resetTo(blockBuilder.getPositionCount() - 1);
-                }
-                else {
+                if (!encountered[fieldIndex]) {
+                    fieldDecoders.get(fieldIndex).decode(ionReader, blockBuilder);
                     encountered[fieldIndex] = true;
                 }
-                fieldDecoders.get(fieldIndex).decode(ionReader, blockBuilder);
             }
 
             for (int i = 0; i < encountered.length; i++) {

Diff for: lib/trino-hive-formats/src/test/java/io/trino/hive/formats/ion/TestIonFormat.java

@@ -292,20 +292,6 @@ public void testStructWithNullAndMissingValues()
                 Arrays.asList(null, null));
     }
 
-    @Test
-    public void testStructWithDuplicateKeys()
-            throws IOException
-    {
-        // this test is not making a value judgement; capturing the last
-        // is not necessarily the "right" behavior. the test just
-        // documents what the behavior is, which is based on the behavior
-        // of the hive serde, and is consistent with the trino json parser.
-        assertValues(
-                RowType.rowType(field("foo", INTEGER)),
-                "{ foo: 17, foo: 31, foo: 53 } { foo: 67 }",
-                List.of(53), List.of(67));
-    }
-
     @Test
     public void testNestedList()
             throws IOException
@@ -330,6 +316,25 @@ public void testNestedStruct()
                 List.of(List.of("Woody", "Guthrie")));
     }
 
+    /**
+     * The Ion Hive SerDe captures the last value for fields with
+     * duplicate keys. There is different behavior for nested Rows,
+     * which you can see below.
+     */
+    @Test
+    public void testTopLevelStructWithDuplicateKeys()
+            throws IOException
+    {
+        assertValues(
+                RowType.rowType(field("foo", INTEGER)),
+                "{ foo: 17, foo: 31, foo: 53 } { foo: 67 }",
+                List.of(53), List.of(67));
+    }
+
+    /**
+     * The Ion Hive SerDe captures the first value for duplicate fields in
+     * nested Rows, so that is what we default to here.
+     */
     @Test
     public void testNestedStructWithDuplicateAndMissingKeys()
             throws IOException
@@ -340,7 +345,7 @@ public void testNestedStructWithDuplicateAndMissingKeys()
                                 field("first", VARCHAR),
                                 field("last", VARCHAR)))),
                 """
-                        { name: { last: Godfrey, last: Guthrie } }
+                        { name: { last: Guthrie, last: Godfrey } }
                         { name: { first: Joan, last: Baez } }
                         """,
                 List.of(Arrays.asList(null, "Guthrie")),