remove special handling for positiondeletestable

apache · Nov 27, 2024 · 13a016c · 13a016c
1 parent a805cd3
commit 13a016c
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 48 deletions.
diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java
@@ -143,10 +143,6 @@ protected Table newTable(TableOperations ops, String tableName) {
     return new BaseTable(ops, tableName);
   }
 
-  public Table underlyingTable() {
-    return lazyTable();
-  }
-
   @Override
   public String name() {
     return name;

diff --git a/core/src/main/java/org/apache/iceberg/TableUtil.java b/core/src/main/java/org/apache/iceberg/TableUtil.java
@@ -23,21 +23,14 @@
 public class TableUtil {
   private TableUtil() {}
 
-  /**
-   * Returns the format version of the given table. Note that subclasses of {@link
-   * BaseMetadataTable} do not have a format version. The only exception to this is {@link
-   * PositionDeletesTable}, where the format version is fetched from the underlying table.
-   */
+  /** Returns the format version of the given table */
   public static int formatVersion(Table table) {
     Preconditions.checkArgument(null != table, "Invalid table: null");
 
-    if (table instanceof SerializableTable) {
-      return formatVersion(((SerializableTable) table).underlyingTable());
-    } else if (table instanceof PositionDeletesTable) {
-      // being able to read the format version from the PositionDeletesTable is mainly needed in
-      // SparkPositionDeletesRewrite when determining whether to rewrite V2 position deletes to DVs
-      return ((PositionDeletesTable) table).table().operations().current().formatVersion();
-    } else if (table instanceof HasTableOperations) {
+    // SerializableMetadataTable is a subclass of SerializableTable but does not support
+    // operations()
+    if (table instanceof HasTableOperations
+        && !(table instanceof SerializableTable.SerializableMetadataTable)) {
       return ((HasTableOperations) table).operations().current().formatVersion();
     } else {
       throw new IllegalArgumentException(

diff --git a/core/src/test/java/org/apache/iceberg/TestTableUtil.java b/core/src/test/java/org/apache/iceberg/TestTableUtil.java
@@ -22,8 +22,6 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.io.File;
-import java.util.Arrays;
-import java.util.stream.Collectors;
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.inmemory.InMemoryCatalog;
@@ -59,7 +57,7 @@ public void nullTable() {
 
   @ParameterizedTest
   @ValueSource(ints = {1, 2, 3})
-  public void formatVersionFromBaseTable(int formatVersion) {
+  public void formatVersionForBaseTable(int formatVersion) {
     Table table =
         catalog.createTable(
             IDENTIFIER,
@@ -71,33 +69,13 @@ public void formatVersionFromBaseTable(int formatVersion) {
     assertThat(TableUtil.formatVersion(SerializableTable.copyOf(table))).isEqualTo(formatVersion);
   }
 
-  @ParameterizedTest
-  @ValueSource(ints = {1, 2, 3})
-  public void formatVersionFromPositionDeletesTable(int formatVersion) {
-    Table table =
-        catalog.createTable(
-            IDENTIFIER,
-            new Schema(Types.NestedField.required(1, "id", Types.IntegerType.get())),
-            PartitionSpec.unpartitioned(),
-            ImmutableMap.of(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion)));
-
-    PositionDeletesTable positionDeletesTable = new PositionDeletesTable(table);
-    assertThat(TableUtil.formatVersion(positionDeletesTable)).isEqualTo(formatVersion);
-    assertThat(TableUtil.formatVersion(SerializableTable.copyOf(positionDeletesTable)))
-        .isEqualTo(formatVersion);
-  }
-
   @Test
   public void formatVersionForMetadataTables() {
     Table table =
         catalog.createTable(
             IDENTIFIER, new Schema(Types.NestedField.required(1, "id", Types.IntegerType.get())));
 
-    for (MetadataTableType type :
-        Arrays.stream(MetadataTableType.values())
-            .filter(type -> type != MetadataTableType.POSITION_DELETES)
-            .collect(Collectors.toList())) {
-
+    for (MetadataTableType type : MetadataTableType.values()) {
       Table metadataTable = MetadataTableUtils.createMetadataTableInstance(table, type);
       assertThatThrownBy(() -> TableUtil.formatVersion(metadataTable))
           .isInstanceOf(IllegalArgumentException.class)
@@ -107,7 +85,8 @@ public void formatVersionForMetadataTables() {
       assertThatThrownBy(() -> TableUtil.formatVersion(SerializableTable.copyOf(metadataTable)))
           .isInstanceOf(IllegalArgumentException.class)
           .hasMessage(
-              "%s does not have a format version", metadataTable.getClass().getSimpleName());
+              "%s does not have a format version",
+              SerializableTable.SerializableMetadataTable.class.getSimpleName());
     }
   }
 }
diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestTableSerialization.java b/core/src/test/java/org/apache/iceberg/hadoop/TestTableSerialization.java
@@ -108,13 +108,9 @@ public void testSerializableMetadataTable() throws IOException, ClassNotFoundExc
       assertThatThrownBy(() -> ((HasTableOperations) serializableTable).operations())
           .isInstanceOf(UnsupportedOperationException.class)
           .hasMessageEndingWith("does not support operations()");
-      if (MetadataTableType.POSITION_DELETES == type) {
-        assertThat(TableUtil.formatVersion(serializableTable)).isEqualTo(2);
-      } else {
-        assertThatThrownBy(() -> TableUtil.formatVersion(serializableTable))
-            .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageEndingWith("does not have a format version");
-      }
+      assertThatThrownBy(() -> TableUtil.formatVersion(serializableTable))
+          .isInstanceOf(IllegalArgumentException.class)
+          .hasMessageEndingWith("does not have a format version");
     }
   }