From 432a442542c159dd6825e28b4b7d0f0cb6253144 Mon Sep 17 00:00:00 2001 From: wgcn <1026688210@qq.com> Date: Tue, 15 Aug 2023 21:45:19 +0800 Subject: [PATCH] [hive] extern table remove file by mistake when create table failed (#1798) --- .../apache/paimon/hive/PaimonMetaHook.java | 12 ++- .../apache/paimon/hive/CreateTableITCase.java | 99 +++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java b/paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java index 69f24f73b6e7..64d359045a26 100644 --- a/paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java +++ b/paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonMetaHook.java @@ -35,7 +35,6 @@ 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; @@ -43,8 +42,10 @@ 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 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 = 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/paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java b/paimon-hive/paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java index fe5c8f84532f..85506453d069 100644 --- a/paimon-hive/paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/CreateTableITCase.java +++ b/paimon-hive/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()); + } + } }