Silently drop tables with missing filesystem metadata

Adds two utility methods to bypass the previous "Table metadata is missing" error thrown when attempting to drop a table for which a metadata file or folder is missing from the underlying file system. The utility methods eat the TableNotFoundException and continue with the drop task. Adds a test for this case as well. Signed-off-by: kiersten-stokes <[email protected]>
prestodb · Oct 25, 2023 · 4d52f63 · 4d52f63
1 parent 7bfbf52
commit 4d52f63
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 7 deletions.
diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java
@@ -132,6 +132,9 @@ public List<ConnectorTableLayoutResult> getTableLayouts(ConnectorSession session
     @Override
     public ConnectorTableLayout getTableLayout(ConnectorSession session, ConnectorTableLayoutHandle handle)
     {
+        IcebergTableHandle tableHandle = ((IcebergTableLayoutHandle) handle).getTable();
+        Table table = getIcebergTable(session, tableHandle.getSchemaTableName());
+        validateTableMode(session, table);
         return new ConnectorTableLayout(handle);
     }
 
@@ -381,6 +384,7 @@ public ConnectorInsertTableHandle beginInsert(ConnectorSession session, Connecto
     {
         IcebergTableHandle table = (IcebergTableHandle) tableHandle;
         Table icebergTable = getIcebergTable(session, table.getSchemaTableName());
+        validateTableMode(session, icebergTable);
 
         return beginIcebergTableInsert(table, icebergTable);
     }
@@ -414,7 +418,6 @@ public IcebergTableHandle getTableHandle(ConnectorSession session, SchemaTableNa
 
         // use a new schema table name that omits the table type
         Table table = getIcebergTable(session, new SchemaTableName(tableName.getSchemaName(), name.getTableName()));
-        validateTableMode(session, table);
 
         return new IcebergTableHandle(
                 tableName.getSchemaName(),

diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergHiveMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergHiveMetadata.java
@@ -14,6 +14,7 @@
 package com.facebook.presto.iceberg;
 
 import com.facebook.airlift.json.JsonCodec;
+import com.facebook.airlift.log.Logger;
 import com.facebook.presto.common.predicate.TupleDomain;
 import com.facebook.presto.common.type.Type;
 import com.facebook.presto.common.type.TypeManager;
@@ -108,6 +109,7 @@
 import static com.facebook.presto.iceberg.IcebergUtil.getHiveIcebergTable;
 import static com.facebook.presto.iceberg.IcebergUtil.getTableComment;
 import static com.facebook.presto.iceberg.IcebergUtil.isIcebergTable;
+import static com.facebook.presto.iceberg.IcebergUtil.tryGetProperties;
 import static com.facebook.presto.iceberg.PartitionFields.parsePartitionFields;
 import static com.facebook.presto.iceberg.util.StatisticsUtil.mergeHiveStatistics;
 import static com.facebook.presto.spi.StandardErrorCode.INVALID_SCHEMA_PROPERTY;
@@ -130,6 +132,7 @@
 public class IcebergHiveMetadata
         extends IcebergAbstractMetadata
 {
+    private static final Logger log = Logger.get(IcebergAbstractMetadata.class);
     private final ExtendedHiveMetastore metastore;
     private final HdfsEnvironment hdfsEnvironment;
     private final String prestoVersion;
@@ -316,11 +319,14 @@ public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle
         IcebergTableHandle handle = (IcebergTableHandle) tableHandle;
         // TODO: support path override in Iceberg table creation
         org.apache.iceberg.Table table = getHiveIcebergTable(metastore, hdfsEnvironment, session, handle.getSchemaTableName());
-        if (table.properties().containsKey(OBJECT_STORE_PATH) ||
-                table.properties().containsKey("write.folder-storage.path") || // Removed from Iceberg as of 0.14.0, but preserved for backward compatibility
-                table.properties().containsKey(WRITE_METADATA_LOCATION) ||
-                table.properties().containsKey(WRITE_DATA_LOCATION)) {
-            throw new PrestoException(NOT_SUPPORTED, "Table " + handle.getSchemaTableName() + " contains Iceberg path override properties and cannot be dropped from Presto");
+        Optional<Map<String, String>> tableProperties = tryGetProperties(table);
+        if (tableProperties.isPresent()) {
+            if (tableProperties.get().containsKey(OBJECT_STORE_PATH) ||
+                    tableProperties.get().containsKey("write.folder-storage.path") || // Removed from Iceberg as of 0.14.0, but preserved for backward compatibility
+                    tableProperties.get().containsKey(WRITE_METADATA_LOCATION) ||
+                    tableProperties.get().containsKey(WRITE_DATA_LOCATION)) {
+                throw new PrestoException(NOT_SUPPORTED, "Table " + handle.getSchemaTableName() + " contains Iceberg path override properties and cannot be dropped from Presto");
+            }
         }
         metastore.dropTable(getMetastoreContext(session), handle.getSchemaName(), handle.getTableName(), true);
     }

diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java
@@ -13,6 +13,7 @@
  */
 package com.facebook.presto.iceberg;
 
+import com.facebook.airlift.log.Logger;
 import com.facebook.presto.common.predicate.TupleDomain;
 import com.facebook.presto.common.type.TypeManager;
 import com.facebook.presto.hive.HdfsContext;
@@ -23,6 +24,7 @@
 import com.facebook.presto.spi.ConnectorSession;
 import com.facebook.presto.spi.PrestoException;
 import com.facebook.presto.spi.SchemaTableName;
+import com.facebook.presto.spi.TableNotFoundException;
 import com.facebook.presto.spi.connector.ConnectorMetadata;
 import com.google.common.collect.ImmutableMap;
 import org.apache.iceberg.BaseTable;
@@ -81,6 +83,7 @@
 public final class IcebergUtil
 {
     private static final Pattern SIMPLE_NAME = Pattern.compile("[a-z][a-z0-9]*");
+    private static final Logger log = Logger.get(IcebergUtil.class);
 
     private IcebergUtil() {}
 
@@ -135,7 +138,7 @@ public static Optional<Long> resolveSnapshotIdByName(Table table, IcebergTableNa
             }
             return name.getSnapshotId();
         }
-        return Optional.ofNullable(table.currentSnapshot()).map(Snapshot::snapshotId);
+        return tryGetCurrentSnapshot(table).map(Snapshot::snapshotId);
     }
 
     public static List<IcebergColumnHandle> getColumns(Schema schema, TypeManager typeManager)
@@ -243,4 +246,26 @@ public static Map<String, String> createIcebergViewProperties(ConnectorSession s
                 .put(TABLE_TYPE_PROP, ICEBERG_TABLE_TYPE_VALUE)
                 .build();
     }
+
+    public static Optional<Map<String, String>> tryGetProperties(Table table)
+    {
+        try {
+            return Optional.ofNullable(table.properties());
+        }
+        catch (TableNotFoundException e) {
+            log.warn(String.format("Unable to fetch properties for table %s: %s", table.name(), e.getMessage()));
+            return Optional.empty();
+        }
+    }
+
+    public static Optional<Snapshot> tryGetCurrentSnapshot(Table table)
+    {
+        try {
+            return Optional.ofNullable(table.currentSnapshot());
+        }
+        catch (TableNotFoundException e) {
+            log.warn(String.format("Unable to fetch snapshot for table %s: %s", table.name(), e.getMessage()));
+            return Optional.empty();
+        }
+    }
 }
diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMetadataListing.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMetadataListing.java
@@ -25,6 +25,7 @@
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import java.io.File;
 import java.nio.file.Path;
 import java.util.Map;
 import java.util.Optional;
@@ -116,6 +117,25 @@ public void testTableDescribing()
         assertQuery("DESCRIBE iceberg.test_schema.iceberg_table1", "VALUES ('_string', 'varchar', '', ''), ('_integer', 'integer', '', '')");
     }
 
+    @Test
+    public void testTableDropWithMissingMetadata()
+    {
+        assertQuerySucceeds("CREATE SCHEMA hive.test_metadata_schema");
+        assertQuerySucceeds("CREATE TABLE iceberg.test_metadata_schema.iceberg_table1 (_string VARCHAR, _integer INTEGER)");
+        assertQuerySucceeds("CREATE TABLE iceberg.test_metadata_schema.iceberg_table2 (_string VARCHAR, _integer INTEGER)");
+        assertQuery("SHOW TABLES FROM iceberg.test_metadata_schema", "VALUES 'iceberg_table1', 'iceberg_table2'");
+
+        File tableMetadataDir = ((DistributedQueryRunner) getQueryRunner()).getCoordinator().getDataDirectory().resolve("iceberg_data").resolve("catalog").resolve("test_metadata_schema").resolve("iceberg_table1").resolve("metadata").toFile();
+        for (File file : tableMetadataDir.listFiles()) {
+            file.delete();
+        }
+        tableMetadataDir.delete();
+
+        assertQueryFails("SELECT * FROM iceberg.test_metadata_schema.iceberg_table1", "Table metadata is missing.");
+        assertQuerySucceeds("DROP TABLE iceberg.test_metadata_schema.iceberg_table1");
+        assertQuery("SHOW TABLES FROM iceberg.test_metadata_schema", "VALUES 'iceberg_table2'");
+    }
+
     @Test
     public void testTableValidation()
     {