From 99b75557723bec21c28458e92caf56d0e839d944 Mon Sep 17 00:00:00 2001 From: Karuppayya Rajendran Date: Wed, 27 Oct 2021 16:38:52 -0700 Subject: [PATCH 1/7] Add UUID to metastore table params (cherry picked from commit f88366ab949b602d975d9fccc414be98d6c5d7fa) --- .../org/apache/iceberg/TableProperties.java | 7 +++++ .../iceberg/hive/HiveTableOperations.java | 8 ++++-- .../apache/iceberg/hive/TestHiveCatalog.java | 26 +++++++++++++++++++ .../iceberg/spark/sql/TestCreateTable.java | 2 ++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableProperties.java b/core/src/main/java/org/apache/iceberg/TableProperties.java index 6aaf0265e1a7..06bc27c0fb68 100644 --- a/core/src/main/java/org/apache/iceberg/TableProperties.java +++ b/core/src/main/java/org/apache/iceberg/TableProperties.java @@ -40,6 +40,13 @@ private TableProperties() { */ public static final String FORMAT_VERSION = "format-version"; + /** + * Reserved table property for UUID. + *

+ * This reserved property is keyword is used to store the UUID of the table. + */ + public static final String UUID = "uuid"; + /** * Reserved Iceberg table properties list. *

diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java index a0bb5c3d5963..3539ae17fa61 100644 --- a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java +++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java @@ -262,7 +262,8 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { Map summary = Optional.ofNullable(metadata.currentSnapshot()) .map(Snapshot::summary) .orElseGet(ImmutableMap::of); - setHmsTableParameters(newMetadataLocation, tbl, metadata.properties(), removedProps, hiveEngineEnabled, summary); + setHmsTableParameters(newMetadataLocation, tbl, metadata.properties(), removedProps, hiveEngineEnabled, summary, + metadata.uuid()); if (!keepHiveStats) { tbl.getParameters().remove(StatsSetupConst.COLUMN_STATS_ACCURATE); @@ -354,7 +355,7 @@ private Table newHmsTable() { private void setHmsTableParameters(String newMetadataLocation, Table tbl, Map icebergTableProps, Set obsoleteProps, boolean hiveEngineEnabled, - Map summary) { + Map summary, String uuid) { Map parameters = Optional.ofNullable(tbl.getParameters()) .orElseGet(Maps::newHashMap); @@ -364,6 +365,9 @@ private void setHmsTableParameters(String newMetadataLocation, Table tbl, Map parameters = hmsTable.getParameters(); + Assert.assertNotNull(parameters.get(TableProperties.UUID)); + } finally { + catalog.dropTable(tableIdentifier); + } + } } diff --git a/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java b/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java index 303cbb5f932b..13ed7819d427 100644 --- a/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java +++ b/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java @@ -23,6 +23,7 @@ import java.util.Map; import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseTable; +import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; @@ -65,6 +66,7 @@ public void testCreateTable() { Assert.assertEquals("Should not be partitioned", 0, table.spec().fields().size()); Assert.assertNull("Should not have the default format set", table.properties().get(TableProperties.DEFAULT_FILE_FORMAT)); + Assert.assertNotNull(((HasTableOperations) table).operations().current().uuid()); } @Test From bc1901a4806333ea64cf8e649ac76154e8a4a3a8 Mon Sep 17 00:00:00 2001 From: Yufei Gu Date: Mon, 17 Jan 2022 21:58:30 -0800 Subject: [PATCH 2/7] Refactor (cherry picked from commit 7d66707dd0c74ca7277653d619cd67d4e6f2fb8b) --- core/src/main/java/org/apache/iceberg/TableProperties.java | 2 +- .../test/java/org/apache/iceberg/spark/sql/TestCreateTable.java | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableProperties.java b/core/src/main/java/org/apache/iceberg/TableProperties.java index 06bc27c0fb68..03637eec814e 100644 --- a/core/src/main/java/org/apache/iceberg/TableProperties.java +++ b/core/src/main/java/org/apache/iceberg/TableProperties.java @@ -43,7 +43,7 @@ private TableProperties() { /** * Reserved table property for UUID. *

- * This reserved property is keyword is used to store the UUID of the table. + * This reserved property is used to store the UUID of the table. */ public static final String UUID = "uuid"; diff --git a/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java b/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java index 13ed7819d427..303cbb5f932b 100644 --- a/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java +++ b/spark/v3.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java @@ -23,7 +23,6 @@ import java.util.Map; import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseTable; -import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; @@ -66,7 +65,6 @@ public void testCreateTable() { Assert.assertEquals("Should not be partitioned", 0, table.spec().fields().size()); Assert.assertNull("Should not have the default format set", table.properties().get(TableProperties.DEFAULT_FILE_FORMAT)); - Assert.assertNotNull(((HasTableOperations) table).operations().current().uuid()); } @Test From a6c30c37de349f245fc291cf6c5b909824139572 Mon Sep 17 00:00:00 2001 From: Yufei Gu Date: Tue, 18 Jan 2022 09:06:26 -0800 Subject: [PATCH 3/7] Fix the test failure. (cherry picked from commit e8b9c435722b079035ba3365aae8ba4326c8ad65) --- .../iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java b/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java index 1df2d28c0b06..938ba1997b28 100644 --- a/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java +++ b/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java @@ -624,7 +624,7 @@ public void testIcebergAndHmsTableProperties() throws Exception { Assert.assertEquals(expectedIcebergProperties, icebergTable.properties()); if (Catalogs.hiveCatalog(shell.getHiveConf(), tableProperties)) { - Assert.assertEquals(10, hmsParams.size()); + Assert.assertEquals(11, hmsParams.size()); Assert.assertEquals("initial_val", hmsParams.get("custom_property")); Assert.assertEquals("TRUE", hmsParams.get(InputFormatConfig.EXTERNAL_TABLE_PURGE)); Assert.assertEquals("TRUE", hmsParams.get("EXTERNAL")); @@ -662,7 +662,7 @@ public void testIcebergAndHmsTableProperties() throws Exception { .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); if (Catalogs.hiveCatalog(shell.getHiveConf(), tableProperties)) { - Assert.assertEquals(13, hmsParams.size()); // 2 newly-added properties + previous_metadata_location prop + Assert.assertEquals(14, hmsParams.size()); // 2 newly-added properties + previous_metadata_location prop Assert.assertEquals("true", hmsParams.get("new_prop_1")); Assert.assertEquals("false", hmsParams.get("new_prop_2")); Assert.assertEquals("new_val", hmsParams.get("custom_property")); From 368f0cb03315f4a08edbde58c2f206192090d726 Mon Sep 17 00:00:00 2001 From: yufeigu Date: Tue, 18 Jan 2022 16:16:33 -0800 Subject: [PATCH 4/7] Not allow to create a table with uuid as a property --- core/src/main/java/org/apache/iceberg/TableProperties.java | 3 ++- .../src/test/java/org/apache/iceberg/TestTableMetadata.java | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/TableProperties.java b/core/src/main/java/org/apache/iceberg/TableProperties.java index 03637eec814e..59ce870b5a1e 100644 --- a/core/src/main/java/org/apache/iceberg/TableProperties.java +++ b/core/src/main/java/org/apache/iceberg/TableProperties.java @@ -54,7 +54,8 @@ private TableProperties() { * The value of these properties are not persisted as a part of the table metadata. */ public static final Set RESERVED_PROPERTIES = ImmutableSet.of( - FORMAT_VERSION + FORMAT_VERSION, + UUID ); public static final String COMMIT_NUM_RETRIES = "commit.retry.num-retries"; diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 97f2d98bac05..a5e5efd72c98 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -884,5 +884,11 @@ public void testNoReservedPropertyForTableMetadataCreation() { "Table properties should not contain reserved properties, but got {format-version=1}", () -> TableMetadata.newTableMetadata(schema, PartitionSpec.unpartitioned(), null, "/tmp", ImmutableMap.of(TableProperties.FORMAT_VERSION, "1"), 1)); + + AssertHelpers.assertThrows("should not allow reserved table property when creating table metadata", + IllegalArgumentException.class, + "Table properties should not contain reserved properties, but got {uuid=uuid}", + () -> TableMetadata.newTableMetadata(schema, PartitionSpec.unpartitioned(), null, "/tmp", + ImmutableMap.of(TableProperties.UUID, "uuid"), 1)); } } From c734c24664f400f4b4337258865ab88451ebfe23 Mon Sep 17 00:00:00 2001 From: yufeigu Date: Tue, 18 Jan 2022 17:28:54 -0800 Subject: [PATCH 5/7] Fix the test failure. --- .../org/apache/iceberg/flink/TestFlinkCatalogTable.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flink/v1.14/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.14/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index 4ecfcb040cbb..b0efe154ce53 100644 --- a/flink/v1.14/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.14/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import org.apache.flink.table.api.DataTypes; @@ -179,10 +178,9 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); - final String uuid = UUID.randomUUID().toString(); - final Map expectedProperties = ImmutableMap.of("uuid", uuid); + final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties() - .set("uuid", uuid) + .set("key", "value") .commit(); Assert.assertEquals(expectedProperties, table("tl").properties()); From b092d29109d1fa1f60697fe0d2f0d00f7f82a7de Mon Sep 17 00:00:00 2001 From: yufeigu Date: Tue, 18 Jan 2022 23:40:22 -0800 Subject: [PATCH 6/7] Fix the test failure. --- .../org/apache/iceberg/flink/TestFlinkCatalogTable.java | 6 ++---- .../org/apache/iceberg/flink/TestFlinkCatalogTable.java | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/flink/v1.12/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.12/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index 7b552fa9ecb3..41908ce39177 100644 --- a/flink/v1.12/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.12/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import org.apache.flink.table.api.DataTypes; @@ -178,10 +177,9 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); - final String uuid = UUID.randomUUID().toString(); - final Map expectedProperties = ImmutableMap.of("uuid", uuid); + final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties() - .set("uuid", uuid) + .set("key", "value") .commit(); Assert.assertEquals(expectedProperties, table("tl").properties()); diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index 4ecfcb040cbb..b0efe154ce53 100644 --- a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import org.apache.flink.table.api.DataTypes; @@ -179,10 +178,9 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); - final String uuid = UUID.randomUUID().toString(); - final Map expectedProperties = ImmutableMap.of("uuid", uuid); + final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties() - .set("uuid", uuid) + .set("key", "value") .commit(); Assert.assertEquals(expectedProperties, table("tl").properties()); From 47fb910cd8f8fcd7284004875930ac9d3cd70525 Mon Sep 17 00:00:00 2001 From: yufeigu Date: Mon, 24 Jan 2022 10:32:33 -0800 Subject: [PATCH 7/7] Resolve the comment. --- .../apache/iceberg/hive/HiveTableOperations.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java index 3539ae17fa61..f1f23a14549d 100644 --- a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java +++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java @@ -262,8 +262,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { Map summary = Optional.ofNullable(metadata.currentSnapshot()) .map(Snapshot::summary) .orElseGet(ImmutableMap::of); - setHmsTableParameters(newMetadataLocation, tbl, metadata.properties(), removedProps, hiveEngineEnabled, summary, - metadata.uuid()); + setHmsTableParameters(newMetadataLocation, tbl, metadata, removedProps, hiveEngineEnabled, summary); if (!keepHiveStats) { tbl.getParameters().remove(StatsSetupConst.COLUMN_STATS_ACCURATE); @@ -353,20 +352,20 @@ private Table newHmsTable() { return newTable; } - private void setHmsTableParameters(String newMetadataLocation, Table tbl, Map icebergTableProps, + private void setHmsTableParameters(String newMetadataLocation, Table tbl, TableMetadata metadata, Set obsoleteProps, boolean hiveEngineEnabled, - Map summary, String uuid) { + Map summary) { Map parameters = Optional.ofNullable(tbl.getParameters()) .orElseGet(Maps::newHashMap); // push all Iceberg table properties into HMS - icebergTableProps.forEach((key, value) -> { + metadata.properties().forEach((key, value) -> { // translate key names between Iceberg and HMS where needed String hmsKey = ICEBERG_TO_HMS_TRANSLATION.getOrDefault(key, key); parameters.put(hmsKey, value); }); - if (uuid != null) { - parameters.put(TableProperties.UUID, uuid); + if (metadata.uuid() != null) { + parameters.put(TableProperties.UUID, metadata.uuid()); } // remove any props from HMS that are no longer present in Iceberg table props