Merge pull request #2 from RMS/Merge-PR550

PARQUET-138: Allow merging more restrictive field in less restrictive…
RMS · Feb 22, 2019 · 6f53a30 · 6f53a30
2 parents e98e7fd + 1416e29
commit 6f53a30
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 10 deletions.
diff --git a/parquet-column/src/main/java/org/apache/parquet/schema/GroupType.java b/parquet-column/src/main/java/org/apache/parquet/schema/GroupType.java
@@ -380,9 +380,6 @@ List<Type> mergeFields(GroupType toMerge, boolean strict) {
       Type merged;
       if (toMerge.containsField(type.getName())) {
         Type fieldToMerge = toMerge.getType(type.getName());
-        if (fieldToMerge.getRepetition().isMoreRestrictiveThan(type.getRepetition())) {
-          throw new IncompatibleSchemaModificationException("repetition constraint is more restrictive: can not merge type " + fieldToMerge + " into " + type);
-        }
         if (type.getOriginalType() != null && fieldToMerge.getOriginalType() != type.getOriginalType()) {
           throw new IncompatibleSchemaModificationException("cannot merge original type " + fieldToMerge.getOriginalType() + " into " + type.getOriginalType());
         }

diff --git a/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java b/parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveType.java
@@ -728,7 +728,8 @@ protected Type union(Type toMerge, boolean strict) {
       }
     }
 
-    Types.PrimitiveBuilder<PrimitiveType> builder = Types.primitive(primitive, toMerge.getRepetition());
+    Repetition repetition = Repetition.leastRestrictive(this.getRepetition(), toMerge.getRepetition());
+    Types.PrimitiveBuilder<PrimitiveType> builder = Types.primitive(primitive, repetition);
 
     if (PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY == primitive) {
       builder.length(length);

diff --git a/parquet-column/src/main/java/org/apache/parquet/schema/Type.java b/parquet-column/src/main/java/org/apache/parquet/schema/Type.java
@@ -20,6 +20,7 @@
 
 import static org.apache.parquet.Preconditions.checkNotNull;
 
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.parquet.io.InvalidRecordException;
@@ -111,6 +112,27 @@ public boolean isMoreRestrictiveThan(Repetition other) {
      */
     abstract public boolean isMoreRestrictiveThan(Repetition other);
 
+    /**
+     * @param repetitions repetitions to traverse
+     * @return the least restrictive repetition of all repetitions provided
+     */
+    public static Repetition leastRestrictive(Repetition... repetitions) {
+      boolean hasOptional = false;
+
+      for (Repetition repetition : repetitions) {
+        if (repetition == REPEATED) {
+          return REPEATED;
+        } else if (repetition == OPTIONAL) {
+          hasOptional = true;
+        }
+      }
+
+      if (hasOptional) {
+        return OPTIONAL;
+      }
+
+      return REQUIRED;
+    }
   }
 
   private final String name;

diff --git a/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java b/parquet-column/src/test/java/org/apache/parquet/schema/TestMessageType.java
@@ -89,12 +89,11 @@ public void testMergeSchema() {
     MessageType t4 = new MessageType("root2",
         new PrimitiveType(REQUIRED, BINARY, "a"));
 
-    try {
-      t3.union(t4);
-      fail("moving from optional to required");
-    } catch (IncompatibleSchemaModificationException e) {
-      assertEquals("repetition constraint is more restrictive: can not merge type required binary a into optional binary a", e.getMessage());
-    }
+    assertEquals(
+        t3.union(t4),
+        new MessageType("root1",
+            new PrimitiveType(OPTIONAL, BINARY, "a"))
+        );
 
     assertEquals(
         t4.union(t3),