diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java index c01787c8ae5a..afb6b9ad72be 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java @@ -524,12 +524,10 @@ public synchronized void dropTable(String databaseName, String tableName, boolea Path tableMetadataDirectory = getTableMetadataDirectory(databaseName, tableName); - // It is safe to delete the whole meta directory for external tables and views - if (!table.getTableType().equals(MANAGED_TABLE.name()) || deleteData) { + if (deleteData) { deleteDirectoryAndSchema(TABLE, tableMetadataDirectory); } else { - // in this case we only want to delete the metadata of a managed table deleteSchemaFile(TABLE, tableMetadataDirectory); deleteTablePrivileges(table); } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java index 5c12795c865c..c6f6760b75d5 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java @@ -82,6 +82,7 @@ import static io.trino.plugin.hive.metastore.StorageFormat.VIEW_STORAGE_FORMAT; import static io.trino.plugin.hive.util.HiveUtil.isHiveSystemSchema; import static io.trino.plugin.hive.util.HiveWriteUtils.getTableDefaultLocation; +import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_FILESYSTEM_ERROR; import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.decodeMaterializedViewData; import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.encodeMaterializedViewData; import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.fromConnectorMaterializedViewDefinition; @@ -96,11 +97,13 @@ import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.SCHEMA_NOT_EMPTY; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; import static java.util.UUID.randomUUID; import static org.apache.hadoop.hive.metastore.TableType.VIRTUAL_VIEW; import static org.apache.iceberg.BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE; import static org.apache.iceberg.BaseMetastoreTableOperations.TABLE_TYPE_PROP; +import static org.apache.iceberg.CatalogUtil.dropTableData; import static org.apache.iceberg.TableMetadata.newTableMetadata; import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT_DEFAULT; import static org.apache.iceberg.TableProperties.OBJECT_STORE_PATH; @@ -302,14 +305,40 @@ public List listTables(ConnectorSession session, Optional new TableNotFoundException(schemaTableName)); + metastore.dropTable( + schemaTableName.getSchemaName(), + schemaTableName.getTableName(), + false /* do not delete data */); + // Use the Iceberg routine for dropping the table data because the data files + // of the Iceberg table may be located in different locations + dropTableData(table.io(), metadata); + deleteTableDirectory(session, metastoreTable); + } + + private void deleteTableDirectory(ConnectorSession session, io.trino.plugin.hive.metastore.Table metastoreTable) + { + Path tablePath = new Path(metastoreTable.getStorage().getLocation()); + try { + FileSystem fileSystem = hdfsEnvironment.getFileSystem(new HdfsContext(session), tablePath); + fileSystem.delete(tablePath, true); + } + catch (IOException e) { + throw new TrinoException( + ICEBERG_FILESYSTEM_ERROR, + format("Failed to delete directory %s of the table %s.%s", tablePath, metastoreTable.getDatabaseName(), metastoreTable.getTableName()), + e); + } } @Override diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java index 4579f9e7a9a0..608a39491cec 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java @@ -1097,6 +1097,9 @@ private void testCreateTableLikeForFormat(IcebergFileFormat otherFormat) File tempDir = getDistributedQueryRunner().getCoordinator().getBaseDataDir().toFile(); String tempDirPath = tempDir.toURI().toASCIIString() + randomTableSuffix(); + // LIKE source INCLUDING PROPERTIES copies all the properties of the source table, including the `location`. + // For this reason the source and the copied table will share the same directory. + // This test does not drop intentionally the created tables to avoid affecting the source table or the information_schema. assertUpdate(format("CREATE TABLE test_create_table_like_original (col1 INTEGER, aDate DATE) WITH(format = '%s', location = '%s', partitioning = ARRAY['aDate'])", format, tempDirPath)); assertEquals(getTablePropertiesString("test_create_table_like_original"), "WITH (\n" + format(" format = '%s',\n", format) + @@ -1107,12 +1110,10 @@ private void testCreateTableLikeForFormat(IcebergFileFormat otherFormat) assertUpdate("CREATE TABLE test_create_table_like_copy0 (LIKE test_create_table_like_original, col2 INTEGER)"); assertUpdate("INSERT INTO test_create_table_like_copy0 (col1, aDate, col2) VALUES (1, CAST('1950-06-28' AS DATE), 3)", 1); assertQuery("SELECT * from test_create_table_like_copy0", "VALUES(1, CAST('1950-06-28' AS DATE), 3)"); - dropTable("test_create_table_like_copy0"); assertUpdate("CREATE TABLE test_create_table_like_copy1 (LIKE test_create_table_like_original)"); assertEquals(getTablePropertiesString("test_create_table_like_copy1"), "WITH (\n" + format(" format = '%s',\n location = '%s'\n)", format, tempDir + "/iceberg_data/tpch/test_create_table_like_copy1")); - dropTable("test_create_table_like_copy1"); assertUpdate("CREATE TABLE test_create_table_like_copy2 (LIKE test_create_table_like_original EXCLUDING PROPERTIES)"); assertEquals(getTablePropertiesString("test_create_table_like_copy2"), "WITH (\n" + @@ -1125,7 +1126,6 @@ private void testCreateTableLikeForFormat(IcebergFileFormat otherFormat) format(" location = '%s',\n", tempDirPath) + " partitioning = ARRAY['adate']\n" + ")"); - dropTable("test_create_table_like_copy3"); assertUpdate(format("CREATE TABLE test_create_table_like_copy4 (LIKE test_create_table_like_original INCLUDING PROPERTIES) WITH (format = '%s')", otherFormat)); assertEquals(getTablePropertiesString("test_create_table_like_copy4"), "WITH (\n" + @@ -1133,9 +1133,6 @@ private void testCreateTableLikeForFormat(IcebergFileFormat otherFormat) format(" location = '%s',\n", tempDirPath) + " partitioning = ARRAY['adate']\n" + ")"); - dropTable("test_create_table_like_copy4"); - - dropTable("test_create_table_like_original"); } private String getTablePropertiesString(String tableName) @@ -3367,4 +3364,16 @@ public void testTargetMaxFileSize() // so just to be safe we check if it is not much bigger .forEach(row -> assertThat((Long) row.getField(0)).isBetween(1L, maxSize.toBytes() * 3)); } + + @Test + public void testDroppingIcebergAndCreatingANewTableWithTheSameNameShouldBePossible() + { + assertUpdate("CREATE TABLE test_iceberg_recreate (a_int) AS VALUES (1)", 1); + assertThat(query("SELECT min(a_int) FROM test_iceberg_recreate")).matches("VALUES 1"); + dropTable("test_iceberg_recreate"); + + assertUpdate("CREATE TABLE test_iceberg_recreate (a_varchar) AS VALUES ('Trino')", 1); + assertThat(query("SELECT min(a_varchar) FROM test_iceberg_recreate")).matches("VALUES CAST('Trino' AS varchar)"); + dropTable("test_iceberg_recreate"); + } } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/DataFileRecord.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/DataFileRecord.java new file mode 100644 index 000000000000..9c1db9802d0e --- /dev/null +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/DataFileRecord.java @@ -0,0 +1,134 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.iceberg; + +import com.google.common.collect.ImmutableMap; +import io.trino.testing.MaterializedRow; + +import java.util.Map; + +import static org.testng.Assert.assertEquals; + +public class DataFileRecord +{ + private final int content; + private final String filePath; + private final String fileFormat; + private final long recordCount; + private final long fileSizeInBytes; + private final Map columnSizes; + private final Map valueCounts; + private final Map nullValueCounts; + private final Map nanValueCounts; + private final Map lowerBounds; + private final Map upperBounds; + + public static DataFileRecord toDataFileRecord(MaterializedRow row) + { + assertEquals(row.getFieldCount(), 14); + return new DataFileRecord( + (int) row.getField(0), + (String) row.getField(1), + (String) row.getField(2), + (long) row.getField(3), + (long) row.getField(4), + row.getField(5) != null ? ImmutableMap.copyOf((Map) row.getField(5)) : null, + row.getField(6) != null ? ImmutableMap.copyOf((Map) row.getField(6)) : null, + row.getField(7) != null ? ImmutableMap.copyOf((Map) row.getField(7)) : null, + row.getField(8) != null ? ImmutableMap.copyOf((Map) row.getField(8)) : null, + row.getField(9) != null ? ImmutableMap.copyOf((Map) row.getField(9)) : null, + row.getField(10) != null ? ImmutableMap.copyOf((Map) row.getField(10)) : null); + } + + private DataFileRecord( + int content, + String filePath, + String fileFormat, + long recordCount, + long fileSizeInBytes, + Map columnSizes, + Map valueCounts, + Map nullValueCounts, + Map nanValueCounts, + Map lowerBounds, + Map upperBounds) + { + this.content = content; + this.filePath = filePath; + this.fileFormat = fileFormat; + this.recordCount = recordCount; + this.fileSizeInBytes = fileSizeInBytes; + this.columnSizes = columnSizes; + this.valueCounts = valueCounts; + this.nullValueCounts = nullValueCounts; + this.nanValueCounts = nanValueCounts; + this.lowerBounds = lowerBounds; + this.upperBounds = upperBounds; + } + + public int getContent() + { + return content; + } + + public String getFilePath() + { + return filePath; + } + + public String getFileFormat() + { + return fileFormat; + } + + public long getRecordCount() + { + return recordCount; + } + + public long getFileSizeInBytes() + { + return fileSizeInBytes; + } + + public Map getColumnSizes() + { + return columnSizes; + } + + public Map getValueCounts() + { + return valueCounts; + } + + public Map getNullValueCounts() + { + return nullValueCounts; + } + + public Map getNanValueCounts() + { + return nanValueCounts; + } + + public Map getLowerBounds() + { + return lowerBounds; + } + + public Map getUpperBounds() + { + return upperBounds; + } +} diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergOrcMetricsCollection.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergOrcMetricsCollection.java index c08849fad82e..58d459761a38 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergOrcMetricsCollection.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergOrcMetricsCollection.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.iceberg; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import io.trino.Session; import io.trino.plugin.hive.HdfsConfig; @@ -49,7 +48,7 @@ import static io.trino.SystemSessionProperties.MAX_DRIVERS_PER_TASK; import static io.trino.SystemSessionProperties.TASK_CONCURRENCY; import static io.trino.SystemSessionProperties.TASK_WRITER_COUNT; -import static io.trino.plugin.iceberg.TestIcebergOrcMetricsCollection.DataFileRecord.toDataFileRecord; +import static io.trino.plugin.iceberg.DataFileRecord.toDataFileRecord; import static io.trino.testing.TestingSession.testSessionBuilder; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNull; @@ -409,117 +408,4 @@ public void testWithTimestamps() assertUpdate("DROP TABLE test_timestamp"); } - - public static class DataFileRecord - { - private final int content; - private final String filePath; - private final String fileFormat; - private final long recordCount; - private final long fileSizeInBytes; - private final Map columnSizes; - private final Map valueCounts; - private final Map nullValueCounts; - private final Map nanValueCounts; - private final Map lowerBounds; - private final Map upperBounds; - - public static DataFileRecord toDataFileRecord(MaterializedRow row) - { - assertEquals(row.getFieldCount(), 14); - return new DataFileRecord( - (int) row.getField(0), - (String) row.getField(1), - (String) row.getField(2), - (long) row.getField(3), - (long) row.getField(4), - row.getField(5) != null ? ImmutableMap.copyOf((Map) row.getField(5)) : null, - row.getField(6) != null ? ImmutableMap.copyOf((Map) row.getField(6)) : null, - row.getField(7) != null ? ImmutableMap.copyOf((Map) row.getField(7)) : null, - row.getField(8) != null ? ImmutableMap.copyOf((Map) row.getField(8)) : null, - row.getField(9) != null ? ImmutableMap.copyOf((Map) row.getField(9)) : null, - row.getField(10) != null ? ImmutableMap.copyOf((Map) row.getField(10)) : null); - } - - private DataFileRecord( - int content, - String filePath, - String fileFormat, - long recordCount, - long fileSizeInBytes, - Map columnSizes, - Map valueCounts, - Map nullValueCounts, - Map nanValueCounts, - Map lowerBounds, - Map upperBounds) - { - this.content = content; - this.filePath = filePath; - this.fileFormat = fileFormat; - this.recordCount = recordCount; - this.fileSizeInBytes = fileSizeInBytes; - this.columnSizes = columnSizes; - this.valueCounts = valueCounts; - this.nullValueCounts = nullValueCounts; - this.nanValueCounts = nanValueCounts; - this.lowerBounds = lowerBounds; - this.upperBounds = upperBounds; - } - - public int getContent() - { - return content; - } - - public String getFilePath() - { - return filePath; - } - - public String getFileFormat() - { - return fileFormat; - } - - public long getRecordCount() - { - return recordCount; - } - - public long getFileSizeInBytes() - { - return fileSizeInBytes; - } - - public Map getColumnSizes() - { - return columnSizes; - } - - public Map getValueCounts() - { - return valueCounts; - } - - public Map getNullValueCounts() - { - return nullValueCounts; - } - - public Map getNanValueCounts() - { - return nanValueCounts; - } - - public Map getLowerBounds() - { - return lowerBounds; - } - - public Map getUpperBounds() - { - return upperBounds; - } - } } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithCustomLocation.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithCustomLocation.java index ce6fb0022053..51c5de0c2264 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithCustomLocation.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithCustomLocation.java @@ -15,10 +15,26 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import io.trino.plugin.hive.HdfsConfig; +import io.trino.plugin.hive.HdfsConfiguration; +import io.trino.plugin.hive.HdfsConfigurationInitializer; +import io.trino.plugin.hive.HdfsEnvironment; +import io.trino.plugin.hive.HdfsEnvironment.HdfsContext; +import io.trino.plugin.hive.HiveHdfsConfiguration; +import io.trino.plugin.hive.NodeVersion; +import io.trino.plugin.hive.authentication.NoHdfsAuthentication; +import io.trino.plugin.hive.metastore.MetastoreConfig; import io.trino.plugin.hive.metastore.Table; import io.trino.plugin.hive.metastore.file.FileHiveMetastore; +import io.trino.plugin.hive.metastore.file.FileHiveMetastoreConfig; +import io.trino.spi.security.ConnectorIdentity; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.DistributedQueryRunner; +import io.trino.testing.MaterializedResult; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.metastore.TableType; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; @@ -29,7 +45,7 @@ import static com.google.common.io.MoreFiles.deleteRecursively; import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; -import static io.trino.plugin.hive.metastore.file.FileHiveMetastore.createTestingFileHiveMetastore; +import static io.trino.plugin.iceberg.DataFileRecord.toDataFileRecord; import static io.trino.plugin.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; @@ -43,13 +59,26 @@ public class TestIcebergTableWithCustomLocation { private FileHiveMetastore metastore; private File metastoreDir; + private HdfsEnvironment hdfsEnvironment; + private HdfsContext hdfsContext; @Override protected DistributedQueryRunner createQueryRunner() throws Exception { metastoreDir = Files.createTempDirectory("test_iceberg").toFile(); - metastore = createTestingFileHiveMetastore(metastoreDir); + HdfsConfig hdfsConfig = new HdfsConfig(); + HdfsConfiguration hdfsConfiguration = new HiveHdfsConfiguration(new HdfsConfigurationInitializer(hdfsConfig), ImmutableSet.of()); + hdfsEnvironment = new HdfsEnvironment(hdfsConfiguration, hdfsConfig, new NoHdfsAuthentication()); + FileHiveMetastoreConfig config = new FileHiveMetastoreConfig() + .setCatalogDirectory(metastoreDir.toURI().toString()) + .setMetastoreUser("test"); + hdfsContext = new HdfsContext(ConnectorIdentity.ofUser(config.getMetastoreUser())); + metastore = new FileHiveMetastore( + new NodeVersion("testversion"), + hdfsEnvironment, + new MetastoreConfig(), + config); return createIcebergQueryRunner( ImmutableMap.of(), @@ -78,14 +107,24 @@ public void testTableHasUuidSuffixInLocation() @Test public void testCreateAndDrop() + throws IOException { String tableName = "test_create_and_drop"; assertQuerySucceeds(format("CREATE TABLE %s as select 1 as val", tableName)); - Optional table = metastore.getTable("tpch", tableName); - assertTrue(table.isPresent(), "Table should exist"); - + Table table = metastore.getTable("tpch", tableName).orElseThrow(); + assertThat(table.getTableType()).isEqualTo(TableType.EXTERNAL_TABLE.name()); + + Path tableLocation = new Path(table.getStorage().getLocation()); + FileSystem fileSystem = hdfsEnvironment.getFileSystem(hdfsContext, tableLocation); + assertTrue(fileSystem.exists(tableLocation), "The directory corresponding to the table storage location should exist"); + MaterializedResult materializedResult = computeActual("SELECT * FROM \"test_create_and_drop$files\""); + assertEquals(materializedResult.getRowCount(), 1); + DataFileRecord dataFile = toDataFileRecord(materializedResult.getMaterializedRows().get(0)); + assertTrue(fileSystem.exists(new Path(dataFile.getFilePath())), "The data file should exist"); assertQuerySucceeds(format("DROP TABLE %s", tableName)); assertFalse(metastore.getTable("tpch", tableName).isPresent(), "Table should be dropped"); + assertFalse(fileSystem.exists(new Path(dataFile.getFilePath())), "The data file should have been removed"); + assertFalse(fileSystem.exists(tableLocation), "The directory corresponding to the dropped Iceberg table should not be removed because it may be shared with other tables"); } @Test diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithExternalLocation.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithExternalLocation.java new file mode 100644 index 000000000000..dabcd49467c2 --- /dev/null +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableWithExternalLocation.java @@ -0,0 +1,122 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.iceberg; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import io.trino.plugin.hive.HdfsConfig; +import io.trino.plugin.hive.HdfsConfiguration; +import io.trino.plugin.hive.HdfsConfigurationInitializer; +import io.trino.plugin.hive.HdfsEnvironment; +import io.trino.plugin.hive.HdfsEnvironment.HdfsContext; +import io.trino.plugin.hive.HiveHdfsConfiguration; +import io.trino.plugin.hive.NodeVersion; +import io.trino.plugin.hive.authentication.NoHdfsAuthentication; +import io.trino.plugin.hive.metastore.MetastoreConfig; +import io.trino.plugin.hive.metastore.Table; +import io.trino.plugin.hive.metastore.file.FileHiveMetastore; +import io.trino.plugin.hive.metastore.file.FileHiveMetastoreConfig; +import io.trino.spi.security.ConnectorIdentity; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.DistributedQueryRunner; +import io.trino.testing.MaterializedResult; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.metastore.TableType; +import org.testng.annotations.AfterClass; +import org.testng.annotations.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.Optional; + +import static com.google.common.io.MoreFiles.deleteRecursively; +import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; +import static io.trino.plugin.iceberg.DataFileRecord.toDataFileRecord; +import static io.trino.plugin.iceberg.IcebergQueryRunner.createIcebergQueryRunner; +import static io.trino.testing.sql.TestTable.randomTableSuffix; +import static java.lang.String.format; +import static org.assertj.core.api.Assertions.assertThat; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +public class TestIcebergTableWithExternalLocation + extends AbstractTestQueryFramework +{ + private FileHiveMetastore metastore; + private File metastoreDir; + private HdfsEnvironment hdfsEnvironment; + private HdfsContext hdfsContext; + + @Override + protected DistributedQueryRunner createQueryRunner() + throws Exception + { + metastoreDir = Files.createTempDirectory("test_iceberg").toFile(); + HdfsConfig hdfsConfig = new HdfsConfig(); + HdfsConfiguration hdfsConfiguration = new HiveHdfsConfiguration(new HdfsConfigurationInitializer(hdfsConfig), ImmutableSet.of()); + hdfsEnvironment = new HdfsEnvironment(hdfsConfiguration, hdfsConfig, new NoHdfsAuthentication()); + FileHiveMetastoreConfig config = new FileHiveMetastoreConfig() + .setCatalogDirectory(metastoreDir.toURI().toString()) + .setMetastoreUser("test"); + hdfsContext = new HdfsContext(ConnectorIdentity.ofUser(config.getMetastoreUser())); + metastore = new FileHiveMetastore( + new NodeVersion("testversion"), + hdfsEnvironment, + new MetastoreConfig(), + config); + + return createIcebergQueryRunner( + ImmutableMap.of(), + ImmutableMap.of(), + ImmutableList.of(), + Optional.of(metastoreDir)); + } + + @AfterClass(alwaysRun = true) + public void tearDown() + throws IOException + { + deleteRecursively(metastoreDir.toPath(), ALLOW_INSECURE); + } + + @Test + public void testCreateAndDrop() + throws IOException + { + String tableName = "test_table_external_create_and_drop"; + File tempDir = getDistributedQueryRunner().getCoordinator().getBaseDataDir().toFile(); + String tempDirPath = tempDir.toURI().toASCIIString() + randomTableSuffix(); + assertQuerySucceeds(format("CREATE TABLE %s ( x bigint) WITH (location = '%s')", tableName, tempDirPath)); + assertQuerySucceeds(format("INSERT INTO %s VALUES (1), (2), (3)", tableName)); + + Table table = metastore.getTable("tpch", tableName).orElseThrow(); + assertThat(table.getTableType()).isEqualTo(TableType.EXTERNAL_TABLE.name()); + Path tableLocation = new Path(table.getStorage().getLocation()); + FileSystem fileSystem = hdfsEnvironment.getFileSystem(hdfsContext, tableLocation); + assertTrue(fileSystem.exists(tableLocation), "The directory corresponding to the table storage location should exist"); + MaterializedResult materializedResult = computeActual("SELECT * FROM \"test_table_external_create_and_drop$files\""); + assertEquals(materializedResult.getRowCount(), 1); + DataFileRecord dataFile = toDataFileRecord(materializedResult.getMaterializedRows().get(0)); + assertTrue(fileSystem.exists(new Path(dataFile.getFilePath())), "The data file should exist"); + + assertQuerySucceeds(format("DROP TABLE %s", tableName)); + assertFalse(metastore.getTable("tpch", tableName).isPresent(), "Table should be dropped"); + assertFalse(fileSystem.exists(new Path(dataFile.getFilePath())), "The data file should have been removed"); + assertFalse(fileSystem.exists(tableLocation), "The directory corresponding to the dropped Iceberg table should not be removed because it may be shared with other tables"); + } +} diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkDropTableCompatibility.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkDropTableCompatibility.java new file mode 100644 index 000000000000..ea1c716d4c71 --- /dev/null +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkDropTableCompatibility.java @@ -0,0 +1,114 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.tests.product.iceberg; + +import com.google.inject.name.Named; +import io.trino.tempto.BeforeTestWithContext; +import io.trino.tempto.ProductTest; +import io.trino.tempto.hadoop.hdfs.HdfsClient; +import io.trino.tests.product.hive.Engine; +import org.assertj.core.api.Assertions; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import javax.inject.Inject; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.List; +import java.util.stream.Collectors; + +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.Engine.SPARK; +import static io.trino.tests.product.hive.Engine.TRINO; +import static io.trino.tests.product.hive.util.TemporaryHiveTable.randomTableSuffix; +import static io.trino.tests.product.utils.QueryExecutors.onSpark; +import static io.trino.tests.product.utils.QueryExecutors.onTrino; +import static java.lang.String.format; + +/** + * Tests drop table compatibility between Iceberg connector and Spark Iceberg. + */ +public class TestIcebergSparkDropTableCompatibility + extends ProductTest +{ + @Inject + @Named("databases.hive.warehouse_directory_path") + private String warehouseDirectory; + + @Inject + private HdfsClient hdfsClient; + + @BeforeTestWithContext + public void useIceberg() + { + onTrino().executeQuery("USE iceberg.default"); + // see spark-defaults.conf + onSpark().executeQuery("USE iceberg_test.default"); + } + + @DataProvider + public static Object[][] tableCleanupEngineConfigurations() + { + return new Object[][] { + {TRINO, TRINO}, + {TRINO, SPARK}, + {SPARK, SPARK}, + {SPARK, TRINO}, + }; + } + + @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS}, dataProvider = "tableCleanupEngineConfigurations") + public void testCleanupOnDropTable(Engine tableCreatorEngine, Engine tableDropperEngine) + { + String tableName = "test_cleanup_on_drop_table" + randomTableSuffix(); + + tableCreatorEngine.queryExecutor().executeQuery("CREATE TABLE " + tableName + "(col0 INT, col1 INT)"); + onTrino().executeQuery("INSERT INTO " + tableName + " VALUES (1, 2)"); + + String tableDirectory = format("%s/%s", warehouseDirectory, tableName); + assertFileExistence(tableDirectory, true, "The table directory exists after creating the table"); + List dataFilePaths = getDataFilePaths(tableName); + + tableDropperEngine.queryExecutor().executeQuery("DROP TABLE " + tableName); + assertFileExistence(tableDirectory, false, format("The table directory %s should be removed after dropping the table", tableDirectory)); + dataFilePaths.forEach(dataFilePath -> assertFileExistence(dataFilePath, false, format("The data file %s removed after dropping the table", dataFilePath))); + } + + private void assertFileExistence(String path, boolean exists, String description) + { + Assertions.assertThat(hdfsClient.exist(path)).as(description).isEqualTo(exists); + } + + private static List getDataFilePaths(String icebergTableName) + { + List filePaths = onTrino().executeQuery(format("SELECT file_path FROM \"%s$files\"", icebergTableName)).column(1); + return filePaths.stream().map(TestIcebergSparkDropTableCompatibility::getPath).collect(Collectors.toList()); + } + + private static String getPath(String uri) + { + if (uri.startsWith("hdfs://")) { + try { + return new URI(uri).getPath(); + } + catch (URISyntaxException e) { + throw new RuntimeException("Invalid syntax for the URI: " + uri, e); + } + } + + return uri; + } +}