[iceberg] branch master updated: API: Add java type to composite TypeID enums (#5154)

[bl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new 9742ea099 API: Add java type to composite TypeID enums (#5154)
9742ea099 is described below

commit 9742ea099a97be049cb382b9c4e81c7fb2e5948d
Author: Dmitri Bourlatchkov <dm...@dremio.com>
AuthorDate: Sun Jul 3 16:14:05 2022 -0400

    API: Add java type to composite TypeID enums (#5154)
---
 .../main/java/org/apache/iceberg/Accessors.java    |  18 +--
 .../java/org/apache/iceberg/types/Comparators.java |  18 +--
 .../main/java/org/apache/iceberg/types/Type.java   |   8 +-
 .../org/apache/iceberg/types/TestComparators.java  | 151 +++++++++++++++++++++
 4 files changed, 158 insertions(+), 37 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/Accessors.java b/api/src/main/java/org/apache/iceberg/Accessors.java
index 04a210987..e18cf3272 100644
--- a/api/src/main/java/org/apache/iceberg/Accessors.java
+++ b/api/src/main/java/org/apache/iceberg/Accessors.java
@@ -63,23 +63,7 @@ public class Accessors {
     PositionAccessor(int pos, Type type) {
       this.position = pos;
       this.type = type;
-
-      switch (type.typeId()) {
-        case MAP:
-          this.javaClass = Map.class;
-          break;
-
-        case LIST:
-          this.javaClass = List.class;
-          break;
-
-        case STRUCT:
-          this.javaClass = StructLike.class;
-          break;
-
-        default:
-          this.javaClass = type.typeId().javaClass();
-      }
+      this.javaClass = type.typeId().javaClass();
     }
 
     @Override
diff --git a/api/src/main/java/org/apache/iceberg/types/Comparators.java b/api/src/main/java/org/apache/iceberg/types/Comparators.java
index 8e477bd73..d876815ea 100644
--- a/api/src/main/java/org/apache/iceberg/types/Comparators.java
+++ b/api/src/main/java/org/apache/iceberg/types/Comparators.java
@@ -22,7 +22,6 @@ package org.apache.iceberg.types;
 import java.nio.ByteBuffer;
 import java.util.Comparator;
 import java.util.List;
-import java.util.Map;
 import java.util.function.IntFunction;
 import org.apache.iceberg.StructLike;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
@@ -84,21 +83,6 @@ public class Comparators {
     throw new UnsupportedOperationException("Cannot determine comparator for type: " + type);
   }
 
-  @SuppressWarnings("unchecked")
-  private static <T> Class<T> internalClass(Type type) {
-    if (type.isPrimitiveType()) {
-      return (Class<T>) type.typeId().javaClass();
-    } else if (type.isStructType()) {
-      return (Class<T>) StructLike.class;
-    } else if (type.isListType()) {
-      return (Class<T>) List.class;
-    } else if (type.isMapType()) {
-      return (Class<T>) Map.class;
-    }
-
-    throw new UnsupportedOperationException("Cannot determine expected class for type: " + type);
-  }
-
   private static class StructLikeComparator implements Comparator<StructLike> {
     private final Comparator<Object>[] comparators;
     private final Class<?>[] classes;
@@ -111,7 +95,7 @@ public class Comparators {
           )
           .toArray((IntFunction<Comparator<Object>[]>) Comparator[]::new);
       this.classes = struct.fields().stream()
-          .map(field -> internalClass(field.type()))
+          .map(field -> field.type().typeId().javaClass())
           .toArray(Class<?>[]::new);
     }
 
diff --git a/api/src/main/java/org/apache/iceberg/types/Type.java b/api/src/main/java/org/apache/iceberg/types/Type.java
index 7c7f97e1b..8a5c9b66a 100644
--- a/api/src/main/java/org/apache/iceberg/types/Type.java
+++ b/api/src/main/java/org/apache/iceberg/types/Type.java
@@ -24,6 +24,8 @@ import java.io.Serializable;
 import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.StructLike;
 
 public interface Type extends Serializable {
   enum TypeID {
@@ -40,9 +42,9 @@ public interface Type extends Serializable {
     FIXED(ByteBuffer.class),
     BINARY(ByteBuffer.class),
     DECIMAL(BigDecimal.class),
-    STRUCT(Void.class),
-    LIST(Void.class),
-    MAP(Void.class);
+    STRUCT(StructLike.class),
+    LIST(List.class),
+    MAP(Map.class);
 
     private final Class<?> javaClass;
 
diff --git a/api/src/test/java/org/apache/iceberg/types/TestComparators.java b/api/src/test/java/org/apache/iceberg/types/TestComparators.java
new file mode 100644
index 000000000..b9b31b579
--- /dev/null
+++ b/api/src/test/java/org/apache/iceberg/types/TestComparators.java
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.types;
+
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.UUID;
+import org.apache.iceberg.TestHelpers;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestComparators {
+
+  private <T> void assertComparesCorrectly(Comparator<T> cmp, T less, T greater) {
+    Assert.assertEquals(0, cmp.compare(greater, greater));
+    Assert.assertEquals(0, cmp.compare(less, less));
+    Assert.assertEquals(-1, Integer.signum(cmp.compare(less, greater)));
+    Assert.assertEquals(1, Integer.signum(cmp.compare(greater, less)));
+  }
+
+  @Test
+  public void testBoolean() {
+    assertComparesCorrectly(Comparators.forType(Types.BooleanType.get()), false, true);
+  }
+
+  @Test
+  public void testInt() {
+    assertComparesCorrectly(Comparators.forType(Types.IntegerType.get()), 0, 1);
+  }
+
+  @Test
+  public void testLong() {
+    assertComparesCorrectly(Comparators.forType(Types.LongType.get()), 0L, 1L);
+  }
+
+  @Test
+  public void testFloat() {
+    assertComparesCorrectly(Comparators.forType(Types.FloatType.get()), 0.1f, 0.2f);
+  }
+
+  @Test
+  public void testDouble() {
+    assertComparesCorrectly(Comparators.forType(Types.DoubleType.get()), 0.1d, 0.2d);
+  }
+
+  @Test
+  public void testDate() {
+    assertComparesCorrectly(Comparators.forType(Types.DateType.get()), 111, 222);
+  }
+
+  @Test
+  public void testTime() {
+    assertComparesCorrectly(Comparators.forType(Types.TimeType.get()), 111, 222);
+  }
+
+  @Test
+  public void testTimestamp() {
+    assertComparesCorrectly(Comparators.forType(Types.TimestampType.withoutZone()), 111, 222);
+    assertComparesCorrectly(Comparators.forType(Types.TimestampType.withZone()), 111, 222);
+  }
+
+  @Test
+  public void testString() {
+    assertComparesCorrectly(Comparators.forType(Types.StringType.get()), "a", "b");
+  }
+
+  @Test
+  public void testUuid() {
+    assertComparesCorrectly(Comparators.forType(Types.UUIDType.get()),
+        UUID.fromString("81873e7d-1374-4493-8e1d-9095eff7046c"),
+        UUID.fromString("fd02441d-1423-4a3f-8785-c7dd5647e26b"));
+  }
+
+  @Test
+  public void testFixed() {
+    assertComparesCorrectly(
+        Comparators.forType(Types.FixedType.ofLength(3)),
+        ByteBuffer.wrap(new byte[] {1, 1, 3}),
+        ByteBuffer.wrap(new byte[] {1, 2, 1}));
+  }
+
+  @Test
+  public void testBinary() {
+    assertComparesCorrectly(
+        Comparators.forType(Types.BinaryType.get()),
+        ByteBuffer.wrap(new byte[] {1, 1}),
+        ByteBuffer.wrap(new byte[] {1, 1, 1}));
+  }
+
+  @Test
+  public void testDecimal() {
+    assertComparesCorrectly(
+        Comparators.forType(Types.DecimalType.of(5, 7)),
+        BigDecimal.valueOf(0.1),
+        BigDecimal.valueOf(0.2));
+  }
+
+  @Test
+  public void testList() {
+    assertComparesCorrectly(
+        Comparators.forType(Types.ListType.ofRequired(18, Types.IntegerType.get())),
+        ImmutableList.of(1, 1, 1),
+        ImmutableList.of(1, 1, 2));
+
+    assertComparesCorrectly(
+        Comparators.forType(Types.ListType.ofRequired(18, Types.IntegerType.get())),
+        ImmutableList.of(1, 1),
+        ImmutableList.of(1, 1, 1));
+
+    assertComparesCorrectly(
+        Comparators.forType(Types.ListType.ofOptional(18, Types.IntegerType.get())),
+        Collections.singletonList(null),
+        Collections.singletonList(1));
+  }
+
+  @Test
+  public void testStruct() {
+    assertComparesCorrectly(
+        Comparators.forType(Types.StructType.of(
+            Types.NestedField.required(18, "str19", Types.StringType.get()),
+            Types.NestedField.required(19, "int19", Types.IntegerType.get()))),
+        TestHelpers.Row.of("a", 1),
+        TestHelpers.Row.of("a", 2));
+
+    assertComparesCorrectly(
+        Comparators.forType(Types.StructType.of(
+            Types.NestedField.optional(18, "str19", Types.StringType.get()))),
+        TestHelpers.Row.of((String) null),
+        TestHelpers.Row.of("a"));
+  }
+}