Update fromPrimitiveString to fromTypeString

apache · Feb 1, 2025 · fc10750 · fc10750
1 parent eb11b53
commit fc10750
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 22 deletions.
diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml
@@ -1146,6 +1146,10 @@ acceptedBreaks:
         \ org.apache.iceberg.TableMetadata)"
       justification: "Removing deprecated code"
   "1.7.0":
+    org.apache.iceberg:iceberg-api:
+    - code: "java.method.removed"
+      old: "method org.apache.iceberg.types.Type.PrimitiveType org.apache.iceberg.types.Types::fromPrimitiveString(java.lang.String)"
+      justification: "Replace with fromTypeString to support variant type"
     org.apache.iceberg:iceberg-core:
     - code: "java.method.removed"
       old: "method <T extends org.apache.iceberg.StructLike> org.apache.iceberg.deletes.PositionDeleteIndex\

diff --git a/api/src/main/java/org/apache/iceberg/types/PrimitiveHolder.java b/api/src/main/java/org/apache/iceberg/types/PrimitiveHolder.java
@@ -33,6 +33,6 @@ class PrimitiveHolder implements Serializable {
   }
 
   Object readResolve() throws ObjectStreamException {
-    return Types.fromPrimitiveString(typeAsString);
+    return Types.fromTypeString(typeAsString);
   }
 }
diff --git a/api/src/main/java/org/apache/iceberg/types/Types.java b/api/src/main/java/org/apache/iceberg/types/Types.java
@@ -63,7 +63,7 @@ private Types() {}
   private static final Pattern DECIMAL =
       Pattern.compile("decimal\\(\\s*(\\d+)\\s*,\\s*(\\d+)\\s*\\)");
 
-  public static Type fromPrimitiveString(String typeString) {
+  public static Type fromTypeString(String typeString) {
     String lowerTypeString = typeString.toLowerCase(Locale.ROOT);
     if (TYPES.containsKey(lowerTypeString)) {
       return TYPES.get(lowerTypeString);

diff --git a/api/src/test/java/org/apache/iceberg/types/TestTypes.java b/api/src/test/java/org/apache/iceberg/types/TestTypes.java
@@ -26,28 +26,27 @@
 public class TestTypes {
 
   @Test
-  public void fromPrimitiveString() {
-    assertThat(Types.fromPrimitiveString("boolean")).isSameAs(Types.BooleanType.get());
-    assertThat(Types.fromPrimitiveString("BooLean")).isSameAs(Types.BooleanType.get());
+  public void fromTypeString() {
+    assertThat(Types.fromTypeString("boolean")).isSameAs(Types.BooleanType.get());
+    assertThat(Types.fromTypeString("BooLean")).isSameAs(Types.BooleanType.get());
 
-    assertThat(Types.fromPrimitiveString("timestamp")).isSameAs(Types.TimestampType.withoutZone());
-    assertThat(Types.fromPrimitiveString("timestamptz")).isSameAs(Types.TimestampType.withZone());
-    assertThat(Types.fromPrimitiveString("timestamp_ns"))
+    assertThat(Types.fromTypeString("timestamp")).isSameAs(Types.TimestampType.withoutZone());
+    assertThat(Types.fromTypeString("timestamptz")).isSameAs(Types.TimestampType.withZone());
+    assertThat(Types.fromTypeString("timestamp_ns"))
         .isSameAs(Types.TimestampNanoType.withoutZone());
-    assertThat(Types.fromPrimitiveString("timestamptz_ns"))
-        .isSameAs(Types.TimestampNanoType.withZone());
+    assertThat(Types.fromTypeString("timestamptz_ns")).isSameAs(Types.TimestampNanoType.withZone());
 
-    assertThat(Types.fromPrimitiveString("Fixed[ 3 ]")).isEqualTo(Types.FixedType.ofLength(3));
+    assertThat(Types.fromTypeString("Fixed[ 3 ]")).isEqualTo(Types.FixedType.ofLength(3));
 
-    assertThat(Types.fromPrimitiveString("Decimal( 2 , 3 )")).isEqualTo(Types.DecimalType.of(2, 3));
+    assertThat(Types.fromTypeString("Decimal( 2 , 3 )")).isEqualTo(Types.DecimalType.of(2, 3));
 
-    assertThat(Types.fromPrimitiveString("Decimal(2,3)")).isEqualTo(Types.DecimalType.of(2, 3));
+    assertThat(Types.fromTypeString("Decimal(2,3)")).isEqualTo(Types.DecimalType.of(2, 3));
 
-    assertThat(Types.fromPrimitiveString("variant")).isSameAs(Types.VariantType.get());
-    assertThat(Types.fromPrimitiveString("Variant")).isSameAs(Types.VariantType.get());
+    assertThat(Types.fromTypeString("variant")).isSameAs(Types.VariantType.get());
+    assertThat(Types.fromTypeString("Variant")).isSameAs(Types.VariantType.get());
 
     assertThatExceptionOfType(IllegalArgumentException.class)
-        .isThrownBy(() -> Types.fromPrimitiveString("abcdefghij"))
+        .isThrownBy(() -> Types.fromTypeString("abcdefghij"))
         .withMessage("Cannot parse type string to primitive: abcdefghij");
   }
 }
diff --git a/core/src/main/java/org/apache/iceberg/SchemaParser.java b/core/src/main/java/org/apache/iceberg/SchemaParser.java
@@ -181,7 +181,7 @@ public static String toJson(Schema schema, boolean pretty) {
 
   private static Type typeFromJson(JsonNode json) {
     if (json.isTextual()) {
-      return Types.fromPrimitiveString(json.asText());
+      return Types.fromTypeString(json.asText());
     } else if (json.isObject()) {
       JsonNode typeObj = json.get(TYPE);
       if (typeObj != null) {

diff --git a/core/src/test/java/org/apache/iceberg/TestSchemaUnionByFieldName.java b/core/src/test/java/org/apache/iceberg/TestSchemaUnionByFieldName.java
@@ -75,7 +75,7 @@ private static NestedField[] primitiveFields(
                 optional(
                     atomicInteger.incrementAndGet(),
                     type.toString(),
-                    Types.fromPrimitiveString(type.toString())))
+                    Types.fromTypeString(type.toString())))
         .toArray(NestedField[]::new);
   }
 

diff --git a/core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java b/core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java
@@ -380,7 +380,7 @@ public void testVariantConversion() {
     org.apache.avro.Schema variantSchema = avroSchema.getField("variantCol").schema();
     assertThat(variantSchema.getType()).isEqualTo(org.apache.avro.Schema.Type.RECORD);
     assertThat(variantSchema.getFields().size()).isEqualTo(2);
-    assertThat(variantSchema.getField("metadata")).isNotNull();
-    assertThat(variantSchema.getField("value")).isNotNull();
+    assertThat(variantSchema.getField("metadata").schema().getType()).isEqualTo(Schema.Type.BYTES);
+    assertThat(variantSchema.getField("value").schema().getType()).isEqualTo(Schema.Type.BYTES);
   }
 }
diff --git a/data/src/test/java/org/apache/iceberg/data/avro/TestGenericData.java b/data/src/test/java/org/apache/iceberg/data/avro/TestGenericData.java
@@ -89,13 +89,13 @@ protected boolean supportsDefaultValues() {
 
   @Test
   public void testSchemaWithTwoVariants() throws JsonProcessingException {
-    final Schema FILE_SCHEMA =
+    final Schema schema =
         new Schema(
             required(1, "id", Types.IntegerType.get()),
             optional(2, "v1", Types.VariantType.get()),
             optional(3, "v2", Types.VariantType.get()));
 
-    GenericRecordBuilder builder = new GenericRecordBuilder(convert(FILE_SCHEMA, "table"));
+    GenericRecordBuilder builder = new GenericRecordBuilder(convert(schema, "table"));
     builder.set("id", 1);
 
     GenericData.Record record = builder.build();