From 009cbeb45e842c51ec221e3ecde01ca76e2c5294 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 24 Mar 2023 14:44:26 +0900 Subject: [PATCH] Allow migrating external tables in Iceberg --- .../iceberg/procedure/MigrateProcedure.java | 4 +- .../iceberg/TestIcebergMigrateProcedure.java | 2 +- .../iceberg/TestIcebergProcedureCalls.java | 44 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/procedure/MigrateProcedure.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/procedure/MigrateProcedure.java index 1a7f9a577d0e..dbe3272fe3ce 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/procedure/MigrateProcedure.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/procedure/MigrateProcedure.java @@ -192,8 +192,8 @@ public void doMigrate(ConnectorSession session, String schemaName, String tableN if (parseBoolean(transactionalProperty)) { throw new TrinoException(NOT_SUPPORTED, "Migrating transactional tables is unsupported"); } - if (!"MANAGED_TABLE".equalsIgnoreCase(hiveTable.getTableType())) { - throw new TrinoException(NOT_SUPPORTED, "The procedure supports migrating only managed tables: " + hiveTable.getTableType()); + if (!"MANAGED_TABLE".equalsIgnoreCase(hiveTable.getTableType()) && !"EXTERNAL_TABLE".equalsIgnoreCase(hiveTable.getTableType())) { + throw new TrinoException(NOT_SUPPORTED, "The procedure doesn't support migrating %s table type".formatted(hiveTable.getTableType())); } if (isDeltaLakeTable(hiveTable)) { throw new TrinoException(NOT_SUPPORTED, "The procedure doesn't support migrating Delta Lake tables"); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMigrateProcedure.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMigrateProcedure.java index abbef53111f5..ec4ff8e6796e 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMigrateProcedure.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMigrateProcedure.java @@ -282,7 +282,7 @@ public void testMigrateUnsupportedTableType() assertQueryFails( "CALL iceberg.system.migrate('tpch', '" + viewName + "')", - "The procedure supports migrating only managed tables: .*"); + "The procedure doesn't support migrating VIRTUAL_VIEW table type"); assertQuery("SELECT * FROM " + trinoViewInHive, "VALUES 1"); assertQuery("SELECT * FROM " + trinoViewInIceberg, "VALUES 1"); diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergProcedureCalls.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergProcedureCalls.java index fcb9ac3b25ef..051c395c5b46 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergProcedureCalls.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergProcedureCalls.java @@ -17,12 +17,16 @@ import org.assertj.core.api.Assertions; import org.testng.annotations.Test; +import java.util.function.Consumer; + import static io.trino.tempto.assertions.QueryAssert.Row.row; import static io.trino.tempto.assertions.QueryAssert.assertQueryFailure; import static io.trino.tempto.assertions.QueryAssert.assertThat; import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.tests.product.TestGroups.ICEBERG; import static io.trino.tests.product.TestGroups.PROFILE_SPECIFIC_TESTS; +import static io.trino.tests.product.hive.util.TableLocationUtils.getTableLocation; +import static io.trino.tests.product.utils.QueryExecutors.onHive; import static io.trino.tests.product.utils.QueryExecutors.onSpark; import static io.trino.tests.product.utils.QueryExecutors.onTrino; import static java.lang.String.format; @@ -122,6 +126,46 @@ public void testMigrateHiveBucketedOnMultipleColumns() onTrino().executeQuery("DROP TABLE IF EXISTS " + hiveTableName); } + @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS}) + public void testTrinoMigrateExternalTable() + { + migrateExternalTable(tableName -> onTrino().executeQuery("CALL iceberg.system.migrate('default', '" + tableName + "')")); + } + + @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS}) + public void testSparkMigrateExternalTable() + { + migrateExternalTable(tableName -> onSpark().executeQuery("CALL iceberg_test.system.migrate('default." + tableName + "')")); + } + + private void migrateExternalTable(Consumer migrateTable) + { + String managedTableName = "test_migrate_managed_" + randomNameSuffix(); + String externalTableName = "test_migrate_external_" + randomNameSuffix(); + String icebergTableName = "iceberg.default." + externalTableName; + String sparkTableName = "iceberg_test.default." + externalTableName; + + onTrino().executeQuery("DROP TABLE IF EXISTS hive.default." + managedTableName); + onTrino().executeQuery("CREATE TABLE hive.default." + managedTableName + " AS SELECT 1 x"); + String tableLocation = getTableLocation("hive.default." + managedTableName); + onTrino().executeQuery("CREATE TABLE hive.default." + externalTableName + "(x integer) WITH (external_location = '" + tableLocation + "')"); + + // Migrate an external table + migrateTable.accept(externalTableName); + + assertThat(onTrino().executeQuery("SELECT * FROM " + icebergTableName)).containsOnly(row(1)); + assertThat(onSpark().executeQuery("SELECT * FROM " + sparkTableName)).containsOnly(row(1)); + + // The migrated table behaves like managed tables because Iceberg doesn't have an external table concept + onTrino().executeQuery("DROP TABLE " + icebergTableName); + + assertQueryFailure(() -> onTrino().executeQuery("SELECT * FROM hive.default." + managedTableName)) + .hasMessageContaining("Partition location does not exist"); + assertThat(onHive().executeQuery("SELECT * FROM default." + managedTableName)).hasNoRows(); + + onTrino().executeQuery("DROP TABLE hive.default." + managedTableName); + } + @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS}) public void testMigrateUnsupportedTransactionalTable() {