diff --git a/core/trino-main/src/main/java/io/trino/execution/DropTableTask.java b/core/trino-main/src/main/java/io/trino/execution/DropTableTask.java index a44e4957d92d..d6187572766a 100644 --- a/core/trino-main/src/main/java/io/trino/execution/DropTableTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/DropTableTask.java @@ -20,16 +20,23 @@ import io.trino.metadata.QualifiedObjectName; import io.trino.metadata.RedirectionAwareTableHandle; import io.trino.security.AccessControl; +import io.trino.spi.TrinoException; +import io.trino.spi.connector.CatalogSchemaTableName; +import io.trino.spi.connector.DropRedirected; +import io.trino.spi.connector.DropResult; import io.trino.sql.tree.DropTable; import io.trino.sql.tree.Expression; import javax.inject.Inject; import java.util.List; +import java.util.Optional; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; +import static io.trino.metadata.QualifiedObjectName.convertFromSchemaTableName; import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR; +import static io.trino.spi.StandardErrorCode.METADATA_NOT_FOUND; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static java.util.Objects.requireNonNull; @@ -76,18 +83,45 @@ public ListenableFuture execute( "Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", originalTableName, originalTableName); } - RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName); - if (redirectionAwareTableHandle.getTableHandle().isEmpty()) { - if (!statement.isExists()) { - throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", originalTableName); + Optional redirectionAwareTableHandle = Optional.empty(); + try { + redirectionAwareTableHandle = Optional.of(metadata.getRedirectionAwareTableHandle(session, originalTableName)); + if (redirectionAwareTableHandle.orElseThrow().getTableHandle().isEmpty()) { + if (!statement.isExists()) { + throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", originalTableName); + } + return immediateVoidFuture(); } - return immediateVoidFuture(); } - QualifiedObjectName tableName = redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName); - accessControl.checkCanDropTable(session.toSecurityContext(), tableName); + catch (TrinoException e) { + if (!e.getErrorCode().equals(METADATA_NOT_FOUND.toErrorCode())) { + throw e; + } + // Ignore Exception when metadata not found and force table drop. + } - metadata.dropTable(session, redirectionAwareTableHandle.getTableHandle().get()); + QualifiedObjectName tableName = originalTableName; + if (redirectionAwareTableHandle.isPresent()) { + tableName = redirectionAwareTableHandle.get().getRedirectedTableName().orElse(originalTableName); + } + + DropResult dropResult = dropTable(session, tableName); + + if (dropResult.isRedirected()) { + // When getRedirectionAwareTableHandle fails to load the table due to metadata missing, and table is redirected. + // we will get the redirected target name from dropResult. + CatalogSchemaTableName catalogSchemaTableName = ((DropRedirected) dropResult).getRedirectedTarget(); + tableName = convertFromSchemaTableName(catalogSchemaTableName.getCatalogName()) + .apply(catalogSchemaTableName.getSchemaTableName()); + dropTable(session, tableName); + } return immediateVoidFuture(); } + + private DropResult dropTable(Session session, QualifiedObjectName tableName) + { + accessControl.checkCanDropTable(session.toSecurityContext(), tableName); + return metadata.dropTable(session, tableName); + } } diff --git a/core/trino-main/src/main/java/io/trino/metadata/Metadata.java b/core/trino-main/src/main/java/io/trino/metadata/Metadata.java index 74c8de3e8fb4..251b9543ed78 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/Metadata.java +++ b/core/trino-main/src/main/java/io/trino/metadata/Metadata.java @@ -30,6 +30,7 @@ import io.trino.spi.connector.ConnectorTableMetadata; import io.trino.spi.connector.Constraint; import io.trino.spi.connector.ConstraintApplicationResult; +import io.trino.spi.connector.DropResult; import io.trino.spi.connector.JoinApplicationResult; import io.trino.spi.connector.JoinStatistics; import io.trino.spi.connector.JoinType; @@ -252,9 +253,9 @@ Optional getTableHandleForExecute( /** * Drops the specified table * - * @throws RuntimeException if the table cannot be dropped or table handle is no longer valid + * @throws RuntimeException if the table cannot be dropped */ - void dropTable(Session session, TableHandle tableHandle); + DropResult dropTable(Session session, QualifiedObjectName tableName); /** * Truncates the specified table diff --git a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java index cc85fbbebd7f..aa2819adacee 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java +++ b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java @@ -61,6 +61,9 @@ import io.trino.spi.connector.ConnectorViewDefinition; import io.trino.spi.connector.Constraint; import io.trino.spi.connector.ConstraintApplicationResult; +import io.trino.spi.connector.DropRedirected; +import io.trino.spi.connector.DropResult; +import io.trino.spi.connector.DropSuccess; import io.trino.spi.connector.JoinApplicationResult; import io.trino.spi.connector.JoinStatistics; import io.trino.spi.connector.JoinType; @@ -758,14 +761,21 @@ public void setTableAuthorization(Session session, CatalogSchemaTableName table, } @Override - public void dropTable(Session session, TableHandle tableHandle) + public DropResult dropTable(Session session, QualifiedObjectName tableName) { - CatalogHandle catalogHandle = tableHandle.getCatalogHandle(); - CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, catalogHandle); + QualifiedObjectName targetTableName = getRedirectedTableName(session, tableName); + if (!targetTableName.equals(tableName)) { + return new DropRedirected(targetTableName.asCatalogSchemaTableName()); + } + CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, tableName.getCatalogName()); ConnectorMetadata metadata = catalogMetadata.getMetadata(session); - Optional tableName = getTableNameIfSystemSecurity(session, catalogMetadata, tableHandle); - metadata.dropTable(session.toConnectorSession(catalogHandle), tableHandle.getConnectorHandle()); - tableName.ifPresent(name -> systemSecurityMetadata.tableDropped(session, name)); + ConnectorSession connectorSession = session.toConnectorSession(catalogMetadata.getCatalogHandle()); + + metadata.dropTable(connectorSession, tableName.asSchemaTableName()); + if (catalogMetadata.getSecurityManagement() == SYSTEM) { + systemSecurityMetadata.tableDropped(session, tableName.asCatalogSchemaTableName()); + } + return DropSuccess.getInstance(); } @Override diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java b/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java index 587c6203bf2b..65c054dc79e2 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java @@ -180,7 +180,13 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { - tables.remove(getTableName(tableHandle)); + dropTable(session, getTableName(tableHandle)); + } + + @Override + public void dropTable(ConnectorSession session, SchemaTableName tableName) + { + tables.remove(tableName); } @Override diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java index 8b5335073433..1bf6adf9ad3d 100644 --- a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java +++ b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java @@ -522,6 +522,9 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) {} + @Override + public void dropTable(ConnectorSession session, SchemaTableName tableName) {} + @Override public void renameTable(ConnectorSession session, ConnectorTableHandle tableHandle, SchemaTableName newTableName) {} diff --git a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java index daca08fc6a55..fa8efd87c629 100644 --- a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java +++ b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java @@ -39,6 +39,9 @@ import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ColumnMetadata; import io.trino.spi.connector.ConnectorTableMetadata; +import io.trino.spi.connector.DropRedirected; +import io.trino.spi.connector.DropResult; +import io.trino.spi.connector.DropSuccess; import io.trino.spi.connector.MaterializedViewNotFoundException; import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.TestingColumnHandle; @@ -302,9 +305,13 @@ public void createTable(Session session, String catalogName, ConnectorTableMetad } @Override - public void dropTable(Session session, TableHandle tableHandle) + public DropResult dropTable(Session session, QualifiedObjectName tableName) { - tables.remove(getTableName(tableHandle)); + ConnectorTableMetadata connectorTableMetadata = tables.remove(tableName.asSchemaTableName()); + if (connectorTableMetadata == null) { + return new DropRedirected(tableName.asCatalogSchemaTableName()); + } + return DropSuccess.getInstance(); } @Override diff --git a/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java index 6dec5788004a..2b4076089191 100644 --- a/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java +++ b/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java @@ -36,6 +36,7 @@ import io.trino.spi.connector.ConnectorTableMetadata; import io.trino.spi.connector.Constraint; import io.trino.spi.connector.ConstraintApplicationResult; +import io.trino.spi.connector.DropResult; import io.trino.spi.connector.JoinApplicationResult; import io.trino.spi.connector.JoinStatistics; import io.trino.spi.connector.JoinType; @@ -318,7 +319,7 @@ public void setTableAuthorization(Session session, CatalogSchemaTableName table, } @Override - public void dropTable(Session session, TableHandle tableHandle) + public DropResult dropTable(Session session, QualifiedObjectName tableName) { throw new UnsupportedOperationException(); } diff --git a/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java index 708409a2acf9..e0f27e212155 100644 --- a/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java +++ b/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java @@ -32,6 +32,7 @@ import io.trino.spi.connector.ConnectorTableMetadata; import io.trino.spi.connector.Constraint; import io.trino.spi.connector.ConstraintApplicationResult; +import io.trino.spi.connector.DropResult; import io.trino.spi.connector.JoinApplicationResult; import io.trino.spi.connector.JoinStatistics; import io.trino.spi.connector.JoinType; @@ -322,9 +323,9 @@ public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle co } @Override - public void dropTable(Session session, TableHandle tableHandle) + public DropResult dropTable(Session session, QualifiedObjectName tableName) { - delegate.dropTable(session, tableHandle); + return delegate.dropTable(session, tableName); } @Override diff --git a/core/trino-spi/src/main/java/io/trino/spi/StandardErrorCode.java b/core/trino-spi/src/main/java/io/trino/spi/StandardErrorCode.java index cf5892e56ca9..a82bd89f98e1 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/StandardErrorCode.java +++ b/core/trino-spi/src/main/java/io/trino/spi/StandardErrorCode.java @@ -189,6 +189,7 @@ public enum StandardErrorCode EXCEEDED_TASK_DESCRIPTOR_STORAGE_CAPACITY(131082, INSUFFICIENT_RESOURCES), UNSUPPORTED_TABLE_TYPE(133001, EXTERNAL), + METADATA_NOT_FOUND(133002, EXTERNAL), /**/; // Connectors can use error codes starting at the range 0x0100_0000 diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java index d0f56c32e95c..37019383809a 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java @@ -353,6 +353,16 @@ default void createTable(ConnectorSession session, ConnectorTableMetadata tableM throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables"); } + /** + * Drops the specified table + * + * @throws RuntimeException if the table cannot be dropped + */ + default void dropTable(ConnectorSession session, SchemaTableName schemaTableName) + { + dropTable(session, getTableHandle(session, schemaTableName)); + } + /** * Drops the specified table * diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/DropRedirected.java b/core/trino-spi/src/main/java/io/trino/spi/connector/DropRedirected.java new file mode 100644 index 000000000000..d53c15a9c573 --- /dev/null +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/DropRedirected.java @@ -0,0 +1,36 @@ +/* + * 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.spi.connector; + +public final class DropRedirected + extends DropResult +{ + private final CatalogSchemaTableName redirectedTarget; + + public DropRedirected(CatalogSchemaTableName target) + { + this.redirectedTarget = target; + } + + public CatalogSchemaTableName getRedirectedTarget() + { + return redirectedTarget; + } + + @Override + public boolean isRedirected() + { + return true; + } +} diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/DropResult.java b/core/trino-spi/src/main/java/io/trino/spi/connector/DropResult.java new file mode 100644 index 000000000000..314e72134ba6 --- /dev/null +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/DropResult.java @@ -0,0 +1,20 @@ +/* + * 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.spi.connector; + +public abstract sealed class DropResult + permits DropSuccess, DropRedirected +{ + public abstract boolean isRedirected(); +} diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/DropSuccess.java b/core/trino-spi/src/main/java/io/trino/spi/connector/DropSuccess.java new file mode 100644 index 000000000000..b7dcbdd44d08 --- /dev/null +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/DropSuccess.java @@ -0,0 +1,36 @@ +/* + * 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.spi.connector; + +public final class DropSuccess + extends DropResult +{ + private static DropSuccess instance; + + private DropSuccess() {} + + public static DropSuccess getInstance() + { + if (instance == null) { + instance = new DropSuccess(); + } + return instance; + } + + @Override + public boolean isRedirected() + { + return false; + } +} diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java index 14c9d0a19cdc..76b0b5d98830 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java @@ -383,6 +383,14 @@ public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle } } + @Override + public void dropTable(ConnectorSession session, SchemaTableName tableName) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + delegate.dropTable(session, tableName); + } + } + @Override public void truncateTable(ConnectorSession session, ConnectorTableHandle tableHandle) { diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java index f322356b383a..36296b1a61d3 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java @@ -208,6 +208,7 @@ import static io.trino.spi.StandardErrorCode.INVALID_ANALYZE_PROPERTY; import static io.trino.spi.StandardErrorCode.INVALID_SCHEMA_PROPERTY; import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; +import static io.trino.spi.StandardErrorCode.METADATA_NOT_FOUND; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.connector.RetryMode.NO_RETRIES; import static io.trino.spi.connector.RowChangeParadigm.DELETE_ROW_AND_INSERT_ROW; @@ -409,8 +410,10 @@ public DeltaLakeTableHandle getTableHandle(ConnectorSession session, SchemaTable } TableSnapshot tableSnapshot = metastore.getSnapshot(tableName, session); - Optional metadata = metastore.getMetadata(tableSnapshot, session); - metadata.ifPresent(metadataEntry -> verifySupportedColumnMapping(getColumnMappingMode(metadataEntry))); + MetadataEntry metadata = metastore.getMetadata(tableSnapshot, session) + .orElseThrow(() -> new TrinoException(METADATA_NOT_FOUND, "Metadata not found in transaction log for table " + tableName)); + + verifySupportedColumnMapping(getColumnMappingMode(metadata)); return new DeltaLakeTableHandle( tableName.getSchemaName(), tableName.getTableName(), @@ -1431,7 +1434,7 @@ public ConnectorTableHandle beginDelete(ConnectorSession session, ConnectorTable handle.getSchemaName(), handle.getTableName(), handle.getLocation(), - Optional.of(handle.getMetadataEntry()), + handle.getMetadataEntry(), handle.getEnforcedPartitionConstraint(), handle.getNonPartitionConstraint(), handle.getProjectedColumns(), @@ -1514,7 +1517,7 @@ public ConnectorTableHandle beginUpdate(ConnectorSession session, ConnectorTable handle.getSchemaName(), handle.getTableName(), handle.getLocation(), - Optional.of(handle.getMetadataEntry()), + handle.getMetadataEntry(), handle.getEnforcedPartitionConstraint(), handle.getNonPartitionConstraint(), handle.getProjectedColumns(), @@ -2050,12 +2053,16 @@ public Optional getInfo(ConnectorTableHandle table) @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { - DeltaLakeTableHandle handle = (DeltaLakeTableHandle) tableHandle; + dropTable(session, ((DeltaLakeTableHandle) tableHandle).getSchemaTableName()); + } - Table table = metastore.getTable(handle.getSchemaName(), handle.getTableName()) - .orElseThrow(() -> new TableNotFoundException(handle.getSchemaTableName())); + @Override + public void dropTable(ConnectorSession session, SchemaTableName tableName) + { + Table table = metastore.getTable(tableName.getSchemaName(), tableName.getTableName()) + .orElseThrow(() -> new TableNotFoundException(tableName)); - metastore.dropTable(session, handle.getSchemaName(), handle.getTableName(), table.getTableType().equals(EXTERNAL_TABLE.toString())); + metastore.dropTable(session, tableName.getSchemaName(), tableName.getTableName(), table.getTableType().equals(EXTERNAL_TABLE.toString())); } @Override @@ -2231,7 +2238,7 @@ public Optional> applyFilter(C tableName.getSchemaName(), tableName.getTableName(), tableHandle.getLocation(), - Optional.of(tableHandle.getMetadataEntry()), + tableHandle.getMetadataEntry(), // Do not simplify the enforced constraint, the connector is guaranteeing the constraint will be applied as is. // The unenforced constraint will still be checked by the engine. tableHandle.getEnforcedPartitionConstraint() @@ -2356,7 +2363,7 @@ public ConnectorAnalyzeMetadata getStatisticsCollectionMetadata(ConnectorSession handle.getSchemaTableName().getSchemaName(), handle.getSchemaTableName().getTableName(), handle.getLocation(), - Optional.of(metadata), + metadata, TupleDomain.all(), TupleDomain.all(), Optional.empty(), diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableHandle.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableHandle.java index d757deac6a42..68aff36958f3 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableHandle.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableHandle.java @@ -18,7 +18,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import io.airlift.units.DataSize; import io.trino.plugin.deltalake.transactionlog.MetadataEntry; -import io.trino.spi.TrinoException; import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ConnectorTableHandle; import io.trino.spi.connector.SchemaTableName; @@ -30,7 +29,6 @@ import java.util.Set; import static com.google.common.base.Preconditions.checkArgument; -import static io.trino.plugin.deltalake.DeltaLakeErrorCode.DELTA_LAKE_INVALID_SCHEMA; import static io.trino.plugin.deltalake.DeltaLakeTableHandle.WriteType.UPDATE; import static java.util.Objects.requireNonNull; @@ -47,7 +45,7 @@ public enum WriteType private final String schemaName; private final String tableName; private final String location; - private final Optional metadataEntry; + private final MetadataEntry metadataEntry; private final TupleDomain enforcedPartitionConstraint; private final TupleDomain nonPartitionConstraint; private final Optional writeType; @@ -72,7 +70,7 @@ public DeltaLakeTableHandle( @JsonProperty("schemaName") String schemaName, @JsonProperty("tableName") String tableName, @JsonProperty("location") String location, - @JsonProperty("metadataEntry") Optional metadataEntry, + @JsonProperty("metadataEntry") MetadataEntry metadataEntry, @JsonProperty("enforcedPartitionConstraint") TupleDomain enforcedPartitionConstraint, @JsonProperty("nonPartitionConstraint") TupleDomain nonPartitionConstraint, @JsonProperty("writeType") Optional writeType, @@ -105,7 +103,7 @@ public DeltaLakeTableHandle( String schemaName, String tableName, String location, - Optional metadataEntry, + MetadataEntry metadataEntry, TupleDomain enforcedPartitionConstraint, TupleDomain nonPartitionConstraint, Optional writeType, @@ -141,7 +139,7 @@ public static DeltaLakeTableHandle forDelete( String schemaName, String tableName, String location, - Optional metadataEntry, + MetadataEntry metadataEntry, TupleDomain enforcedConstraint, TupleDomain unenforcedConstraint, Optional> projectedColumns, @@ -168,7 +166,7 @@ public static DeltaLakeTableHandle forUpdate( String schemaName, String tableName, String location, - Optional metadataEntry, + MetadataEntry metadataEntry, TupleDomain enforcedConstraint, TupleDomain unenforcedConstraint, Optional> projectedColumns, @@ -200,7 +198,7 @@ public DeltaLakeTableHandle withProjectedColumns(Set projectedColu getSchemaName(), getTableName(), getLocation(), - Optional.of(getMetadataEntry()), + getMetadataEntry(), getEnforcedPartitionConstraint(), getNonPartitionConstraint(), getWriteType(), @@ -253,7 +251,7 @@ public String getLocation() @JsonProperty public MetadataEntry getMetadataEntry() { - return metadataEntry.orElseThrow(() -> new TrinoException(DELTA_LAKE_INVALID_SCHEMA, "Metadata not found in transaction log for " + tableName)); + return metadataEntry; } @JsonProperty diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeMetadata.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeMetadata.java index dc221fde2597..eee6d1cd5edb 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeMetadata.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeMetadata.java @@ -313,32 +313,6 @@ public void testGetInsertLayoutTableUnpartitioned() .isNotPresent(); } - @Test - public void testGetInsertLayoutTableNotFound() - { - SchemaTableName schemaTableName = newMockSchemaTableName(); - - DeltaLakeTableHandle missingTableHandle = new DeltaLakeTableHandle( - schemaTableName.getSchemaName(), - schemaTableName.getTableName(), - getTableLocation(schemaTableName), - Optional.empty(), - TupleDomain.none(), - TupleDomain.none(), - Optional.empty(), - Optional.empty(), - Optional.empty(), - Optional.empty(), - Optional.empty(), - 0, - false); - - assertThatThrownBy(() -> deltaLakeMetadataFactory.create(SESSION.getIdentity()) - .getInsertLayout(SESSION, missingTableHandle)) - .isInstanceOf(TrinoException.class) - .hasMessage("Metadata not found in transaction log for " + schemaTableName.getTableName()); - } - @DataProvider public Object[][] testApplyProjectionProvider() { @@ -505,9 +479,9 @@ private static List createNewColumnAssignments(Map createMetadataEntry() + private static MetadataEntry createMetadataEntry() { - return Optional.of(new MetadataEntry( + return new MetadataEntry( "test_id", "test_name", "test_description", @@ -515,7 +489,7 @@ private static Optional createMetadataEntry() "test_schema", ImmutableList.of("test_partition_column"), ImmutableMap.of("test_configuration_key", "test_configuration_value"), - 1)); + 1); } private String getTableLocation(SchemaTableName schemaTableName) diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSplitManager.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSplitManager.java index 6b7f4aee6b07..8749b4bd5b43 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSplitManager.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSplitManager.java @@ -68,7 +68,7 @@ public class TestDeltaLakeSplitManager "schema", "table", "location", - Optional.of(metadataEntry), + metadataEntry, TupleDomain.all(), TupleDomain.all(), Optional.empty(), diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestTransactionLogAccess.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestTransactionLogAccess.java index b951de0c99ee..98cba2bed3e6 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestTransactionLogAccess.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestTransactionLogAccess.java @@ -134,11 +134,20 @@ private void setupTransactionLogAccess(String tableName, Path tableLocation, Del accessTrackingFileSystemFactory, new ParquetReaderConfig()); + String schemaName = "schema"; DeltaLakeTableHandle tableHandle = new DeltaLakeTableHandle( - "schema", + schemaName, tableName, "location", - Optional.empty(), // ignored + new MetadataEntry( + "id", + tableName, + "a table", + new MetadataEntry.Format("provider", ImmutableMap.of()), + schemaName, + ImmutableList.of(), + ImmutableMap.of(), + 0L), TupleDomain.none(), TupleDomain.none(), Optional.empty(), diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreStatistics.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreStatistics.java index 151e685b0bd8..8868149d5e79 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreStatistics.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/TestDeltaLakeMetastoreStatistics.java @@ -154,7 +154,7 @@ private DeltaLakeTableHandle registerTable(String tableName, String directoryNam "db_name", tableName, "location", - Optional.of(new MetadataEntry("id", "test", "description", null, "", ImmutableList.of(), ImmutableMap.of(), 0)), + new MetadataEntry("id", "test", "description", null, "", ImmutableList.of(), ImmutableMap.of(), 0), TupleDomain.all(), TupleDomain.all(), Optional.empty(), @@ -284,7 +284,7 @@ public void testStatisticsMultipleFiles() tableHandle.getSchemaName(), tableHandle.getTableName(), tableHandle.getLocation(), - Optional.of(tableHandle.getMetadataEntry()), + tableHandle.getMetadataEntry(), TupleDomain.all(), TupleDomain.withColumnDomains(ImmutableMap.of((DeltaLakeColumnHandle) COLUMN_HANDLE, Domain.singleValue(DOUBLE, 42.0))), tableHandle.getWriteType(), @@ -308,7 +308,7 @@ public void testStatisticsNoRecords() tableHandle.getSchemaName(), tableHandle.getTableName(), tableHandle.getLocation(), - Optional.of(tableHandle.getMetadataEntry()), + tableHandle.getMetadataEntry(), TupleDomain.none(), TupleDomain.all(), tableHandle.getWriteType(), @@ -322,7 +322,7 @@ public void testStatisticsNoRecords() tableHandle.getSchemaName(), tableHandle.getTableName(), tableHandle.getLocation(), - Optional.of(tableHandle.getMetadataEntry()), + tableHandle.getMetadataEntry(), TupleDomain.all(), TupleDomain.none(), tableHandle.getWriteType(), diff --git a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java index cc047b7c5014..bc12d7102b72 100644 --- a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java +++ b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java @@ -111,7 +111,7 @@ public void dropPointsTable() Metadata metadata = queryRunner.getMetadata(); Optional tableHandle = metadata.getTableHandle(transactionSession, QualifiedObjectName.valueOf("memory.default.points")); assertTrue(tableHandle.isPresent(), "Table memory.default.points does not exist"); - metadata.dropTable(transactionSession, tableHandle.get()); + metadata.dropTable(transactionSession, QualifiedObjectName.valueOf("memory.default.points")); return null; }); } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java index f041f9ba2dce..4476167cf971 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java @@ -1429,11 +1429,16 @@ public void setColumnComment(ConnectorSession session, ConnectorTableHandle tabl @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { - HiveTableHandle handle = (HiveTableHandle) tableHandle; - if (metastore.getTable(handle.getSchemaName(), handle.getTableName()).isEmpty()) { - throw new TableNotFoundException(handle.getSchemaTableName()); + dropTable(session, ((HiveTableHandle) tableHandle).getSchemaTableName()); + } + + @Override + public void dropTable(ConnectorSession session, SchemaTableName tableName) + { + if (metastore.getTable(tableName.getSchemaName(), tableName.getTableName()).isEmpty()) { + throw new TableNotFoundException(tableName); } - metastore.dropTable(session, handle.getSchemaName(), handle.getTableName()); + metastore.dropTable(session, tableName.getSchemaName(), tableName.getTableName()); } @Override diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java index f006ae07ae65..48c05a897e1e 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java @@ -139,6 +139,7 @@ import org.apache.iceberg.types.Types.StringType; import org.apache.iceberg.types.Types.StructType; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayDeque; @@ -230,6 +231,7 @@ import static io.trino.plugin.iceberg.procedure.IcebergTableProcedureId.REMOVE_ORPHAN_FILES; import static io.trino.spi.StandardErrorCode.INVALID_ANALYZE_PROPERTY; import static io.trino.spi.StandardErrorCode.INVALID_ARGUMENTS; +import static io.trino.spi.StandardErrorCode.METADATA_NOT_FOUND; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.connector.MaterializedViewFreshness.Freshness.FRESH; import static io.trino.spi.connector.MaterializedViewFreshness.Freshness.STALE; @@ -355,6 +357,15 @@ public IcebergTableHandle getTableHandle( catch (TableNotFoundException e) { return null; } + catch (RuntimeException e) { + if (e.getCause() != null) { + if (e.getCause() instanceof FileNotFoundException + || e.getCause().getMessage().contains("The specified key does not exist")) { + throw new TrinoException(METADATA_NOT_FOUND, "Metadata not found in metadata location for table " + tableName, e); + } + } + throw e; + } Optional tableSnapshotId; Schema tableSchema; @@ -1410,7 +1421,13 @@ public Optional getInfo(ConnectorTableHandle tableHandle) @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { - catalog.dropTable(session, ((IcebergTableHandle) tableHandle).getSchemaTableName()); + dropTable(session, ((IcebergTableHandle) tableHandle).getSchemaTableName()); + } + + @Override + public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) + { + catalog.dropTable(session, schemaTableName); } @Override diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index b023a42b3b44..6d1ae20c54ab 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -93,6 +93,7 @@ import static io.trino.plugin.hive.util.HiveUtil.isIcebergTable; import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_BAD_DATA; import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_CATALOG_ERROR; +import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_INVALID_METADATA; import static io.trino.plugin.iceberg.IcebergMaterializedViewAdditionalProperties.STORAGE_SCHEMA; import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.encodeMaterializedViewData; import static io.trino.plugin.iceberg.IcebergMaterializedViewDefinition.fromConnectorMaterializedViewDefinition; @@ -344,16 +345,53 @@ public Table loadTable(ConnectorSession session, SchemaTableName table) @Override public void dropTable(ConnectorSession session, SchemaTableName schemaTableName) { - BaseTable table = (BaseTable) loadTable(session, schemaTableName); - validateTableCanBeDropped(table); + com.amazonaws.services.glue.model.Table metastoreTable; + try { + metastoreTable = stats.getGetTable().call(() -> glueClient + .getTable(new GetTableRequest() + .withDatabaseName(schemaTableName.getSchemaName()) + .withName(schemaTableName.getTableName())) + .getTable()); + } + catch (EntityNotFoundException e) { + throw new TableNotFoundException(schemaTableName, e); + } + + BaseTable table = null; + try { + table = (BaseTable) loadTable(session, schemaTableName); + } + catch (RuntimeException e) { + LOG.warn(e, "Failed to load table " + schemaTableName); + } + + if (table != null) { + validateTableCanBeDropped(table); + } + + String metadataLocation = metastoreTable.getParameters().get(METADATA_LOCATION_PROP); + + if (metadataLocation == null) { + throw new TrinoException(ICEBERG_INVALID_METADATA, format("Table %s is missing [%s] property", schemaTableName, METADATA_LOCATION_PROP)); + } + try { deleteTable(schemaTableName.getSchemaName(), schemaTableName.getTableName()); } catch (AmazonServiceException e) { throw new TrinoException(HIVE_METASTORE_ERROR, e); } - dropTableData(table.io(), table.operations().current()); - deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, table.location()); + + if (table != null) { + try { + dropTableData(table.io(), table.operations().current()); + } + catch (RuntimeException e) { + LOG.warn(e, "Failed to delete data files referenced by TableMetadata for table " + schemaTableName); + } + } + String tableLocation = metadataLocation.replaceFirst("/[^/]*/[^/]*$", ""); + deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, tableLocation); } @Override diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java index 45de0751e5f9..a19aea80f0a9 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java @@ -303,19 +303,37 @@ public List listTables(ConnectorSession session, Optional new TableNotFoundException(schemaTableName)); + + BaseTable table = null; + try { + table = (BaseTable) loadTable(session, schemaTableName); + } + catch (RuntimeException e) { + log.warn(e, "Failed to load table " + schemaTableName); + } + + if (table != null) { + validateTableCanBeDropped(table); + } + 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); + + if (table != null) { + try { + // 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(), table.operations().current()); + } + catch (RuntimeException e) { + log.warn(e, "Failed to delete data files referenced by TableMetadata for table " + schemaTableName); + } + } deleteTableDirectory(fileSystemFactory.create(session), schemaTableName, metastoreTable.getStorage().getLocation()); } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java index 01f5ed24ffcc..dca117add6b6 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java @@ -14,10 +14,16 @@ package io.trino.plugin.iceberg; import com.google.common.collect.ImmutableList; +import io.trino.filesystem.FileIterator; +import io.trino.filesystem.TrinoFileSystem; +import io.trino.filesystem.hdfs.HdfsFileSystemFactory; import io.trino.testing.BaseConnectorSmokeTest; import io.trino.testing.TestingConnectorBehavior; +import io.trino.testing.TestingConnectorSession; import io.trino.testing.sql.TestTable; import org.apache.iceberg.FileFormat; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableMetadataParser; import org.testng.annotations.Test; import java.util.List; @@ -28,22 +34,26 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static io.airlift.concurrent.MoreFutures.getFutureValue; +import static io.trino.plugin.hive.HiveTestUtils.HDFS_ENVIRONMENT; import static io.trino.testing.TestingNames.randomNameSuffix; import static java.lang.String.format; import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newFixedThreadPool; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; public abstract class BaseIcebergConnectorSmokeTest extends BaseConnectorSmokeTest { protected final FileFormat format; + private final TrinoFileSystem trinoFileSystem; public BaseIcebergConnectorSmokeTest(FileFormat format) { this.format = requireNonNull(format, "format is null"); + this.trinoFileSystem = new HdfsFileSystemFactory(HDFS_ENVIRONMENT).create(TestingConnectorSession.SESSION); } @SuppressWarnings("DuplicateBranchesInSwitch") @@ -303,6 +313,86 @@ public void testRegisterTableWithMetadataFile() assertUpdate(format("DROP TABLE %s", tableName)); } + @Test + public void testDropTableWithMissingMetadataFile() + throws Exception + { + String tableName = "test_drop_table_with_missing_metadata_file_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 x, 'INDIA' y", 1); + + String metadataLocation = getMetadataLocation(tableName); + + // Delete current metadata file + trinoFileSystem.deleteFile(metadataLocation); + assertThat(trinoFileSystem.newInputFile(metadataLocation).exists()).as("Current metadata file should not exist").isFalse(); + + // try to drop table + assertUpdate("DROP TABLE " + tableName); + assertFalse(getQueryRunner().tableExists(getSession(), tableName)); + } + + @Test + public void testDropTableWithMissingSnaphsotFile() + throws Exception + { + String tableName = "test_drop_table_with_missing_snapshot_file_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 x, 'INDIA' y", 1); + + String metadataLocation = getMetadataLocation(tableName); + TableMetadata tableMetadata = TableMetadataParser.read(trinoFileSystem.toFileIo(), metadataLocation); + String currentSnapshotFile = tableMetadata.currentSnapshot().manifestListLocation(); + + // Delete current snapshot file + trinoFileSystem.deleteFile(currentSnapshotFile); + assertThat(trinoFileSystem.newInputFile(currentSnapshotFile).exists()).as("Current snapshot file should not exist").isFalse(); + + // try to drop table + assertUpdate("DROP TABLE " + tableName); + assertFalse(getQueryRunner().tableExists(getSession(), tableName)); + } + + @Test + public void testDropTableWithMissingManifestListFile() + throws Exception + { + String tableName = "test_drop_table_with_missing_manifest_list_file_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 x, 'INDIA' y", 1); + + String metadataLocation = getMetadataLocation(tableName); + TableMetadata tableMetadata = TableMetadataParser.read(trinoFileSystem.toFileIo(), metadataLocation); + String manifestListFile = tableMetadata.currentSnapshot().allManifests(trinoFileSystem.toFileIo()).get(0).path(); + + // Delete Manifest List file + trinoFileSystem.deleteFile(manifestListFile); + assertThat(trinoFileSystem.newInputFile(manifestListFile).exists()).as("Manifest list file should not exist").isFalse(); + + // try to drop table + assertUpdate("DROP TABLE " + tableName); + assertFalse(getQueryRunner().tableExists(getSession(), tableName)); + } + + @Test + public void testDropTableWithMissingDataFile() + throws Exception + { + String tableName = "test_drop_table_with_missing_data_file_" + randomNameSuffix(); + assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 x, 'INDIA' y", 1); + + String tableLocation = getTableLocation(tableName); + String tableDataPath = String.format("%s/%s", tableLocation, "data"); + FileIterator fileIterator = trinoFileSystem.listFiles(tableDataPath); + assertTrue(fileIterator.hasNext()); + String dataFile = fileIterator.next().path(); + + // Delete data file + trinoFileSystem.deleteFile(dataFile); + assertThat(trinoFileSystem.newInputFile(dataFile).exists()).as("Data list file should not exist").isFalse(); + + // try to drop table + assertUpdate("DROP TABLE " + tableName); + assertFalse(getQueryRunner().tableExists(getSession(), tableName)); + } + private String getTableLocation(String tableName) { return (String) computeScalar("SELECT DISTINCT regexp_replace(\"$path\", '/[^/]*/[^/]*$', '') FROM " + tableName); diff --git a/testing/trino-benchmark/src/test/java/io/trino/benchmark/MemoryLocalQueryRunner.java b/testing/trino-benchmark/src/test/java/io/trino/benchmark/MemoryLocalQueryRunner.java index 1b010819ded8..2f7ebb7f1fe5 100644 --- a/testing/trino-benchmark/src/test/java/io/trino/benchmark/MemoryLocalQueryRunner.java +++ b/testing/trino-benchmark/src/test/java/io/trino/benchmark/MemoryLocalQueryRunner.java @@ -130,7 +130,7 @@ public void dropTable(String tableName) Metadata metadata = localQueryRunner.getMetadata(); Optional tableHandle = metadata.getTableHandle(session, QualifiedObjectName.valueOf(tableName)); assertTrue(tableHandle.isPresent(), "Table " + tableName + " does not exist"); - metadata.dropTable(session, tableHandle.get()); + metadata.dropTable(session, QualifiedObjectName.valueOf(tableName)); } @Override diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveRedirectionToIceberg.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveRedirectionToIceberg.java index 24144f50d5e8..a5c3860ea869 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveRedirectionToIceberg.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveRedirectionToIceberg.java @@ -13,15 +13,21 @@ */ package io.trino.tests.product.hive; +import io.trino.plugin.hive.metastore.thrift.ThriftMetastoreClient; +import io.trino.tempto.AfterTestWithContext; import io.trino.tempto.BeforeTestWithContext; import io.trino.tempto.ProductTest; import io.trino.tempto.assertions.QueryAssert; +import io.trino.tempto.hadoop.hdfs.HdfsClient; import io.trino.tempto.query.QueryResult; +import org.apache.thrift.TException; import org.assertj.core.api.AbstractStringAssert; import org.assertj.core.api.Assertions; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import javax.inject.Inject; + import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.tempto.assertions.QueryAssert.Row.row; import static io.trino.tempto.assertions.QueryAssert.assertQueryFailure; @@ -30,6 +36,7 @@ import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.tests.product.TestGroups.HIVE_ICEBERG_REDIRECTIONS; import static io.trino.tests.product.TestGroups.PROFILE_SPECIFIC_TESTS; +import static io.trino.tests.product.iceberg.util.IcebergTestUtils.stripNamenodeURI; import static io.trino.tests.product.utils.QueryExecutors.onTrino; import static java.lang.String.format; import static java.sql.JDBCType.VARCHAR; @@ -37,12 +44,27 @@ public class TestHiveRedirectionToIceberg extends ProductTest { + @Inject + private HdfsClient hdfsClient; + @Inject + private TestHiveMetastoreClientFactory testHiveMetastoreClientFactory; + private ThriftMetastoreClient metastoreClient; + @BeforeTestWithContext public void createAdditionalSchema() + throws TException { + this.metastoreClient = testHiveMetastoreClientFactory.createMetastoreClient(); onTrino().executeQuery("CREATE SCHEMA IF NOT EXISTS hive.nondefaultschema"); } + @AfterTestWithContext + public void tearDown() + { + metastoreClient.close(); + metastoreClient = null; + } + @Test(groups = {HIVE_ICEBERG_REDIRECTIONS, PROFILE_SPECIFIC_TESTS}) public void testRedirect() { @@ -614,6 +636,32 @@ public void testDeny() onTrino().executeQuery("DROP TABLE " + icebergTableName); } + @Test(groups = {HIVE_ICEBERG_REDIRECTIONS, PROFILE_SPECIFIC_TESTS}) + public void testDropTableWithMissingMetadataFile() + throws TException + { + String tableName = "test_drop_table_with_missing_metadata_file_" + randomNameSuffix(); + String hiveTableName = "hive.default." + tableName; + String icebergTableName = "iceberg.default." + tableName; + + createIcebergTable(icebergTableName, false); + + assertResultsEqual( + onTrino().executeQuery("TABLE " + hiveTableName), + onTrino().executeQuery("TABLE " + icebergTableName)); + + String metadataLocation = stripNamenodeURI(metastoreClient.getTable("default", tableName).getParameters().get("metadata_location")); + + // Delete current metadata file + hdfsClient.delete(metadataLocation); + Assertions.assertThat(hdfsClient.exist(metadataLocation)).as("Current metadata file should not exist").isFalse(); + + // try to drop table + onTrino().executeQuery("DROP TABLE " + hiveTableName); + assertQueryFailure(() -> onTrino().executeQuery("TABLE " + icebergTableName)) + .hasMessageMatching("\\QQuery failed (#\\E\\S+\\Q): line 1:1: Table '" + icebergTableName + "' does not exist"); + } + private static void createIcebergTable(String tableName, boolean partitioned) { createIcebergTable(tableName, partitioned, true); diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java index 85c6259c1133..0c5ce88e504c 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java @@ -903,7 +903,7 @@ public void testCreateAndDropTableWithSameLocationFailsOnTrino(int specVersion) onTrino().executeQuery(format("DROP TABLE %s", trinoTableName(tableSameLocation1))); assertQueryFailure(() -> onTrino().executeQuery(format("SELECT * FROM %s", trinoTableName(tableSameLocation2)))) - .hasMessageMatching(".*Failed to open input stream for file.*"); + .hasMessageMatching(".*Metadata not found in metadata location for table (.*).*"); // Can't clean up tableSameLocation2 as all data and metadata has been removed }