From bdfc988adca4c9bb8645465e3889292e29fd2cae Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Wed, 27 Nov 2024 08:55:39 +0100 Subject: [PATCH] remove special handling for positiondeletestable --- .../org/apache/iceberg/SerializableTable.java | 4 --- .../java/org/apache/iceberg/TableUtil.java | 15 ++-------- .../org/apache/iceberg/TestTableUtil.java | 29 +++---------------- .../hadoop/TestTableSerialization.java | 10 ++----- 4 files changed, 10 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java index 079eb6ce44e0..a2c0d776423c 100644 --- 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 index b61c0067fb3f..ef753b0ca5f1 100644 --- a/core/src/main/java/org/apache/iceberg/TableUtil.java +++ b/core/src/main/java/org/apache/iceberg/TableUtil.java @@ -23,21 +23,12 @@ 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) { + 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 index de2a4c24fbb7..d33ecb1a91dd 100644 --- 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 index 7ca62b9b7026..247535c58b75 100644 --- 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"); } }