fix: addressing reviews v3

vibhatha · Jul 23, 2024 · 20ed1fc · 20ed1fc
1 parent 37a7341
commit 20ed1fc
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 20 deletions.
diff --git a/dev/archery/archery/integration/datagen.py b/dev/archery/archery/integration/datagen.py
@@ -1896,9 +1896,10 @@ def _temp_path():
         generate_map_case(),
 
         generate_non_canonical_map_case()
-        .skip_tester('Java')  # TODO(ARROW-8715)
+        # .skip_tester('Java')  # TODO(ARROW-8715)
         # Canonical map names are restored on import, so the schemas are unequal
-        .skip_format(SKIP_C_SCHEMA, 'C++'),
+        .skip_format(SKIP_C_SCHEMA, 'C++')
+        .skip_format(SKIP_C_SCHEMA, 'Java'),
 
         generate_nested_case(),
 

diff --git a/java/vector/src/main/codegen/templates/UnionMapWriter.java b/java/vector/src/main/codegen/templates/UnionMapWriter.java
@@ -142,25 +142,25 @@ public UnionMapWriter value() {
 
   private String getWriteFieldName() {
     Field mapField = this.vector.getField();
-    if (mapField == null) {
-      throw new UnsupportedOperationException("MapVector does not have a field.");
-    }
-    if (mapField.getChildren().size() != 1) {
-      throw new UnsupportedOperationException("MapVector does not have a single struct field.");
-    }
+    Preconditions.checkNotNull(mapField, "MapVector does not have a field.");
+    Preconditions.checkArgument(mapField.getChildren().size() == 1,
+        "MapVector does not have a single struct field.");
     Field structField = mapField.getChildren().get(0);
     switch (mode) {
       case KEY:
-        if (structField.getChildren().size() == 2) {
-          return structField.getChildren().get(0).getName();
-        } else {
+        if (structField.getChildren().size() == 0) {
+          // key is not defined in the struct, use default name
           return MapVector.KEY_NAME;
+        } else {
+          return structField.getChildren().get(0).getName();
         }
       case VALUE:
-        if (structField.getChildren().size() == 2) {
-          return structField.getChildren().get(1).getName();
-        } else {
+        if (structField.getChildren().size() < 2) {
+          // key may or may not have been defined in the struct, but
+          // value has not been defined.
           return MapVector.VALUE_NAME;
+        } else {
+          return structField.getChildren().get(1).getName();
         }
       default:
         throw new UnsupportedOperationException("Cannot get field name in OFF mode");

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java
@@ -46,9 +46,9 @@
 public class MapVector extends ListVector {
 
   /** The default name of the key field in the MapVector. */
-  public static String KEY_NAME = "key";
+  public static final String KEY_NAME = "key";
   /** The default name of the value field in the MapVector. */
-  public static String VALUE_NAME = "value";
+  public static final String VALUE_NAME = "value";
 
   public static final String DATA_VECTOR_NAME = "entries";
 

diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestMapVector.java
@@ -23,6 +23,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -1208,15 +1209,13 @@ public void testMakeTransferPairPreserveNullability() {
 
   @Test
   public void testValidateKeyValueFieldNames() {
-    FieldType keyType = new FieldType(false, MinorType.BIGINT.getType(), null, null);
+    FieldType keyType = FieldType.notNullable(MinorType.BIGINT.getType());
     FieldType valueType = FieldType.nullable(MinorType.FLOAT8.getType());
 
     Field keyField = new Field("myKey", keyType, null);
     Field valueField = new Field("myValue", valueType, null);
 
-    List<Field> structFields = new ArrayList<>(2);
-    structFields.add(keyField);
-    structFields.add(valueField);
+    List<Field> structFields = Arrays.asList(keyField, valueField);
 
     Field structField =
         new Field("entry", FieldType.notNullable(ArrowType.Struct.INSTANCE), structFields);