diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/Storage.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/Storage.java index 43992d705bcb..8e42a3689048 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/Storage.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/Storage.java @@ -65,8 +65,10 @@ public Optional getOptionalLocation() public String getLocation() { - // Default getter requires location to be set. Location is not set only in rare case when in CREATE TABLE flow in Hive connector when - // delegate-transactional-managed-table-location-to-metastore is set to true and location is determined by HMS. + // Default getter requires location to be set. Location is not set in the following scenarios: + // 1. CREATE TABLE flow in Hive connector sets delegate-transactional-managed-table-location-to-metastore to true and location is determined by HMS. + // 2. Iceberg format tables + // 3. Delta format tables (sometimes) return location.orElseThrow(); } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java index 007495f7fadd..328682970a37 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java @@ -143,6 +143,7 @@ import static io.trino.spi.security.PrincipalType.USER; import static java.util.Comparator.comparing; import static java.util.Objects.requireNonNull; +import static java.util.function.Predicate.not; import static java.util.function.UnaryOperator.identity; import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toMap; @@ -578,9 +579,10 @@ public void dropTable(String databaseName, String tableName, boolean deleteData) throw new TrinoException(HIVE_METASTORE_ERROR, e); } - String tableLocation = table.getStorage().getLocation(); - if (deleteData && isManagedTable(table) && !isNullOrEmpty(tableLocation)) { - deleteDir(hdfsContext, hdfsEnvironment, new Path(tableLocation), true); + Optional location = table.getStorage().getOptionalLocation() + .filter(not(String::isEmpty)); + if (deleteData && isManagedTable(table) && location.isPresent()) { + deleteDir(hdfsContext, hdfsEnvironment, new Path(location.get()), true); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java index 5b727408bc3a..329b27822881 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/converter/GlueToTrinoConverter.java @@ -83,14 +83,15 @@ public static Table convertTable(com.amazonaws.services.glue.model.Table glueTab .setViewOriginalText(Optional.ofNullable(glueTable.getViewOriginalText())) .setViewExpandedText(Optional.ofNullable(glueTable.getViewExpandedText())); - if (isIcebergTable(tableParameters) || isDeltaLakeTable(tableParameters)) { - // Iceberg and Delta Lake tables do not use the StorageDescriptor field, but we need to return a Table so the caller can check that - // the table is an Iceberg/Delta table and decide whether to redirect or fail. + StorageDescriptor sd = glueTable.getStorageDescriptor(); + + if (isIcebergTable(tableParameters) || (sd == null && isDeltaLakeTable(tableParameters))) { + // Iceberg tables do not need to read the StorageDescriptor field, but we still need to return dummy properties for compatibility + // Delta Lake tables only need to provide a dummy properties if a StorageDescriptor was not explicitly configured. tableBuilder.setDataColumns(ImmutableList.of(new Column("dummy", HIVE_INT, Optional.empty()))); tableBuilder.getStorageBuilder().setStorageFormat(StorageFormat.fromHiveStorageFormat(HiveStorageFormat.PARQUET)); } else { - StorageDescriptor sd = glueTable.getStorageDescriptor(); if (sd == null) { throw new TrinoException(HIVE_UNSUPPORTED_FORMAT, format("Table StorageDescriptor is null for table %s.%s (%s)", dbName, glueTable.getName(), glueTable)); } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java index 87d3702a14ab..e2f9b074ec76 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestGlueToTrinoConverter.java @@ -34,6 +34,7 @@ import java.util.Optional; import static com.amazonaws.util.CollectionUtils.isNullOrEmpty; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.trino.plugin.hive.HiveType.HIVE_STRING; import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestColumn; import static io.trino.plugin.hive.metastore.glue.TestingMetastoreObjects.getGlueTestDatabase; @@ -215,6 +216,15 @@ public void testTableNullParameters() assertNotNull(trinoTable.getStorage().getSerdeParameters()); } + @Test + public void testIcebergTableNullStorageDescriptor() + { + testTable.setParameters(ImmutableMap.of(ICEBERG_TABLE_TYPE_NAME, ICEBERG_TABLE_TYPE_VALUE)); + testTable.setStorageDescriptor(null); + io.trino.plugin.hive.metastore.Table trinoTable = GlueToTrinoConverter.convertTable(testTable, testDatabase.getName()); + assertEquals(trinoTable.getDataColumns().size(), 1); + } + @Test public void testIcebergTableNonNullStorageDescriptor() { @@ -224,13 +234,28 @@ public void testIcebergTableNonNullStorageDescriptor() assertEquals(trinoTable.getDataColumns().size(), 1); } + @Test + public void testDeltaTableNullStorageDescriptor() + { + testTable.setParameters(ImmutableMap.of(SPARK_TABLE_PROVIDER_KEY, DELTA_LAKE_PROVIDER)); + testTable.setStorageDescriptor(null); + io.trino.plugin.hive.metastore.Table trinoTable = GlueToTrinoConverter.convertTable(testTable, testDatabase.getName()); + assertEquals(trinoTable.getDataColumns().size(), 1); + } + @Test public void testDeltaTableNonNullStorageDescriptor() { testTable.setParameters(ImmutableMap.of(SPARK_TABLE_PROVIDER_KEY, DELTA_LAKE_PROVIDER)); assertNotNull(testTable.getStorageDescriptor()); io.trino.plugin.hive.metastore.Table trinoTable = GlueToTrinoConverter.convertTable(testTable, testDatabase.getName()); - assertEquals(trinoTable.getDataColumns().size(), 1); + assertEquals( + trinoTable.getDataColumns().stream() + .map(Column::getName) + .collect(toImmutableSet()), + testTable.getStorageDescriptor().getColumns().stream() + .map(com.amazonaws.services.glue.model.Column::getName) + .collect(toImmutableSet())); } @Test