[hive] extern table remove file by mistake when create table failed (a…

…pache#1798)
zhu3pang · Aug 15, 2023 · 432a442 · 432a442
1 parent 38eb10d
commit 432a442
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 3 deletions.
diff --git a/...ive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java b/...ive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java
@@ -35,16 +35,17 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.HiveMetaHook;
-import org.apache.hadoop.hive.metastore.MetaStoreUtils;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.hadoop.hive.metastore.api.Table;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.apache.hadoop.hive.metastore.Warehouse.getDnsPath;
@@ -62,6 +63,9 @@ public class PaimonMetaHook implements HiveMetaHook {
     private static final String COMMENT = "comment";
     private final Configuration conf;
 
+    // paimon table existed before create hive table
+    private final Set<Identifier> existingPaimonTable = new HashSet<>();
+
     public PaimonMetaHook(Configuration conf) {
         this.conf = conf;
     }
@@ -75,12 +79,12 @@ public void preCreateTable(Table table) throws MetaException {
         table.getSd().setInputFormat(PaimonInputFormat.class.getCanonicalName());
         table.getSd().setOutputFormat(PaimonOutputFormat.class.getCanonicalName());
         String location = LocationKeyExtractor.getPaimonLocation(conf, table);
+        Identifier identifier = Identifier.create(table.getDbName(), table.getTableName());
         if (location == null) {
             String warehouse = conf.get(HiveConf.ConfVars.METASTOREWAREHOUSE.varname);
             org.apache.hadoop.fs.Path hadoopPath =
                     getDnsPath(new org.apache.hadoop.fs.Path(warehouse), conf);
             warehouse = hadoopPath.toUri().toString();
-            Identifier identifier = Identifier.create(table.getDbName(), table.getTableName());
             location = AbstractCatalog.dataTableLocation(warehouse, identifier).toUri().toString();
             table.getSd().setLocation(location);
         }
@@ -97,6 +101,7 @@ public void preCreateTable(Table table) throws MetaException {
         SchemaManager schemaManager = new SchemaManager(fileIO, path);
         Optional<TableSchema> tableSchema = schemaManager.latest();
         if (tableSchema.isPresent()) {
+            existingPaimonTable.add(identifier);
             // paimon table already exists
             return;
         }
@@ -143,7 +148,8 @@ public void preCreateTable(Table table) throws MetaException {
 
     @Override
     public void rollbackCreateTable(Table table) throws MetaException {
-        if (!MetaStoreUtils.isExternalTable(table)) {
+        Identifier identifier = Identifier.create(table.getDbName(), table.getTableName());
+        if (existingPaimonTable.contains(identifier)) {
             return;
         }
 

diff --git a/.../paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java b/.../paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java
@@ -31,8 +31,13 @@
 import org.apache.paimon.shade.guava30.com.google.common.collect.Lists;
 import org.apache.paimon.shade.guava30.com.google.common.collect.Maps;
 
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.HiveMetaHook;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
 import org.apache.hadoop.hive.ql.parse.ParseException;
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
 import java.util.Arrays;
@@ -288,4 +293,98 @@ public void testCreateTableIsPaimonSystemTable() {
                 .hasMessageContaining(
                         "cannot recognize input near 'test' '$' 'schema' in table name");
     }
+
+    @Test
+    public void testCreateTableFailing() throws Exception {
+        // Create a extern table
+
+        {
+            String tableName = "tes1";
+
+            Schema schema =
+                    new Schema(
+                            Lists.newArrayList(
+                                    new DataField(0, "col1", DataTypes.INT(), "first comment"),
+                                    new DataField(1, "col2", DataTypes.STRING(), "second comment"),
+                                    new DataField(
+                                            2, "col3", DataTypes.DECIMAL(5, 3), "last comment"),
+                                    new DataField(
+                                            3, "col4", DataTypes.DECIMAL(5, 3), "last comment")),
+                            Collections.emptyList(),
+                            Collections.emptyList(),
+                            Maps.newHashMap(),
+                            "");
+            Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
+            Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
+            new SchemaManager(LocalFileIO.create(), tablePath).createTable(schema);
+
+            String hiveSql =
+                    String.join(
+                            "\n",
+                            Arrays.asList(
+                                    "CREATE EXTERNAL TABLE " + tableName + " ",
+                                    "STORED BY '" + MockPaimonStorageHandler.class.getName() + "'",
+                                    "LOCATION '" + tablePath.toUri().toString() + "'"));
+            try {
+                hiveShell.execute(hiveSql);
+            } catch (Throwable ignore) {
+            } finally {
+                boolean isPresent =
+                        new SchemaManager(LocalFileIO.create(), tablePath).latest().isPresent();
+                Assertions.assertThat(isPresent).isTrue();
+            }
+        }
+
+        {
+            String tableName = "tes2";
+
+            hiveShell.execute("SET hive.metastore.warehouse.dir=" + path);
+            String hiveSql =
+                    String.join(
+                            "\n",
+                            Arrays.asList(
+                                    "CREATE TABLE " + tableName + " (",
+                                    "user_id "
+                                            + TypeInfoFactory.longTypeInfo.getTypeName()
+                                            + " COMMENT 'The user_id field',",
+                                    "hh "
+                                            + TypeInfoFactory.stringTypeInfo.getTypeName()
+                                            + " COMMENT 'The hh field'",
+                                    ")",
+                                    "STORED BY '"
+                                            + MockPaimonStorageHandler.class.getName()
+                                            + "'"));
+            try {
+                hiveShell.execute(hiveSql);
+            } catch (Exception ignore) {
+            } finally {
+                Identifier identifier = Identifier.create(DATABASE_TEST, tableName);
+                Path tablePath = AbstractCatalog.dataTableLocation(path, identifier);
+                boolean isPresent =
+                        new SchemaManager(LocalFileIO.create(), tablePath).latest().isPresent();
+                Assertions.assertThat(isPresent).isFalse();
+            }
+        }
+    }
+
+    /** Mock create table failed. */
+    public static class MockPaimonMetaHook extends PaimonMetaHook {
+
+        public MockPaimonMetaHook(Configuration conf) {
+            super(conf);
+        }
+
+        @Override
+        public void commitCreateTable(Table table) throws MetaException {
+            throw new RuntimeException("mock create table failed");
+        }
+    }
+
+    /** StorageHanlder with MockPaimonMetaHook. */
+    public static class MockPaimonStorageHandler extends PaimonStorageHandler {
+        @Override
+        public HiveMetaHook getMetaHook() {
+            return new MockPaimonMetaHook(getConf());
+        }
+    }
 }