This is an automated email from the ASF dual-hosted git repository. ngangam pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push: new 5430dda HIVE-24625 CTAS with TBLPROPERTIES ('transactional'='false') loads data into incorrect directory (amagyar, rajkrrsingh) 5430dda is described below commit 5430dda6f259635d12f6ffee34455c24c73a7082 Author: zeroflag <amag...@hortonworks.com> AuthorDate: Tue Jan 12 14:05:58 2021 +0100 HIVE-24625 CTAS with TBLPROPERTIES ('transactional'='false') loads data into incorrect directory (amagyar, rajkrrsingh) --- .../metastore/TestHiveMetastoreTransformer.java | 25 +++++++++++++ .../hive/ql/ddl/table/create/CreateTableDesc.java | 11 +++++- .../ddl/table/create/like/CreateTableLikeDesc.java | 11 +++++- .../create/show/ShowCreateTableOperation.java | 3 +- .../desc/formatter/TextDescTableFormatter.java | 4 +++ .../hadoop/hive/ql/parse/SemanticAnalyzer.java | 19 ++++++---- .../org/apache/hadoop/hive/ql/plan/PlanUtils.java | 7 ++++ .../metastore/api/hive_metastoreConstants.java | 2 ++ .../apache/hadoop/hive/metastore/HMSHandler.java | 4 +++ .../metastore/MetastoreDefaultTransformer.java | 42 ++++++++++++---------- 10 files changed, 100 insertions(+), 28 deletions(-) diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java index 4eb55e0..2d7eeed 100644 --- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java +++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java @@ -48,6 +48,7 @@ import org.apache.hadoop.conf.Configuration; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -560,6 +561,30 @@ public class TestHiveMetastoreTransformer { } } + + @Test + public void testLeavesCtasTableAlone() throws Exception { + try { + resetHMSClient(); + final String dbName = "db1"; + String basetblName = "oldstylemgdtable"; + Map<String, Object> tProps = new HashMap<>(); + String tblName = basetblName; + tProps.put("DBNAME", dbName); + tProps.put("TBLNAME", tblName); + tProps.put("TBLTYPE", TableType.MANAGED_TABLE); + tProps.put("PROPERTIES", TABLE_IS_CTAS + "=true;transactional=false"); + createTableWithCapabilities(tProps); + Table tbl2 = client.getTable(dbName, tblName); + assertEquals(TableType.MANAGED_TABLE.name(), tbl2.getTableType()); + } catch (Exception e) { + e.printStackTrace(); + fail("testTransformerManagedTable failed with " + e.getMessage()); + } finally { + resetHMSClient(); + } + } + @Test public void testTransformerVirtualView() throws Exception { try { diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableDesc.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableDesc.java index affb1af..f89fcdd 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableDesc.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableDesc.java @@ -18,8 +18,11 @@ package org.apache.hadoop.hive.ql.ddl.table.create; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; + import java.io.Serializable; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -488,11 +491,17 @@ public class CreateTableDesc implements DDLDesc, Serializable { /** * @return the table properties */ - @Explain(displayName = "table properties") public Map<String, String> getTblProps() { return tblProps; } + @Explain(displayName = "table properties") + public Map<String, String> getTblPropsExplain() { // only for displaying plan + HashMap<String, String> copy = new HashMap<>(tblProps); + copy.remove(TABLE_IS_CTAS); + return copy; + } + /** * @param tblProps * the table properties to set diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/like/CreateTableLikeDesc.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/like/CreateTableLikeDesc.java index d191dc5..3e850f0 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/like/CreateTableLikeDesc.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/like/CreateTableLikeDesc.java @@ -18,7 +18,10 @@ package org.apache.hadoop.hive.ql.ddl.table.create.like; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; + import java.io.Serializable; +import java.util.HashMap; import java.util.Map; import org.apache.hadoop.hive.ql.ddl.DDLDesc; @@ -107,11 +110,17 @@ public class CreateTableLikeDesc implements DDLDesc, Serializable { return likeTableName; } - @Explain(displayName = "table properties") public Map<String, String> getTblProps() { return tblProps; } + @Explain(displayName = "table properties") + public Map<String, String> getTblPropsExplain() { + HashMap<String, String> copy = new HashMap<>(tblProps); + copy.remove(TABLE_IS_CTAS); + return copy; + } + @Explain(displayName = "isTemporary", displayOnlyOnTrue = true) public boolean isTemporary() { return isTemporary; diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java index 782a3db..aa6236e 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hive.ql.ddl.table.create.show; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.META_TABLE_STORAGE; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import java.io.DataOutputStream; import java.io.IOException; @@ -347,7 +348,7 @@ public class ShowCreateTableOperation extends DDLOperation<ShowCreateTableDesc> } private static final Set<String> PROPERTIES_TO_IGNORE_AT_TBLPROPERTIES = Sets.union( - ImmutableSet.<String>of("TEMPORARY", "EXTERNAL", "comment", "SORTBUCKETCOLSPREFIX", META_TABLE_STORAGE), + ImmutableSet.<String>of("TEMPORARY", "EXTERNAL", "comment", "SORTBUCKETCOLSPREFIX", META_TABLE_STORAGE, TABLE_IS_CTAS), new HashSet<String>(StatsSetupConst.TABLE_PARAMS_STATS_KEYS)); private String getProperties(Table table) { diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java index 25f2d9b..c5c6370 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java @@ -64,6 +64,7 @@ import java.util.TreeMap; import java.util.Map.Entry; import java.util.stream.Collectors; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.apache.hadoop.hive.ql.ddl.ShowUtils.ALIGNMENT; import static org.apache.hadoop.hive.ql.ddl.ShowUtils.DEFAULT_STRINGBUILDER_SIZE; import static org.apache.hadoop.hive.ql.ddl.ShowUtils.FIELD_DELIM; @@ -341,6 +342,9 @@ class TextDescTableFormatter extends DescTableFormatter { Collections.sort(keys); for (String key : keys) { String value = params.get(key); + if (TABLE_IS_CTAS.equals(key)) { + continue; + } if (key.equals(StatsSetupConst.NUM_ERASURE_CODED_FILES)) { if ("0".equals(value)) { continue; diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java index 2041cc7..d15e46a 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java @@ -22,6 +22,7 @@ import static java.util.Objects.nonNull; import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.DYNAMICPARTITIONCONVERT; import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVE_DEFAULT_STORAGE_HANDLER; import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVESTATSDBCLASS; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.apache.hadoop.hive.ql.optimizer.calcite.translator.ASTConverter.NON_FK_FILTERED; import java.io.FileNotFoundException; @@ -13162,12 +13163,13 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { * * @param tblProp * property map + * @param isCTAS * @return Modified table property map */ private Map<String, String> validateAndAddDefaultProperties( - Map<String, String> tblProp, boolean isExt, StorageFormat storageFormat, - String qualifiedTableName, List<Order> sortCols, boolean isMaterialization, - boolean isTemporaryTable, boolean isTransactional, boolean isManaged) throws SemanticException { + Map<String, String> tblProp, boolean isExt, StorageFormat storageFormat, + String qualifiedTableName, List<Order> sortCols, boolean isMaterialization, + boolean isTemporaryTable, boolean isTransactional, boolean isManaged, boolean isCTAS) throws SemanticException { Map<String, String> retValue = Optional.ofNullable(tblProp).orElseGet(HashMap::new); String paraString = HiveConf.getVar(conf, ConfVars.NEWTABLEDEFAULTPARA); @@ -13217,6 +13219,9 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { retValue = convertToAcidByDefault(storageFormat, qualifiedTableName, sortCols, retValue); } } + if (isCTAS) { + retValue.put(TABLE_IS_CTAS, Boolean.toString(isCTAS)); + } return retValue; } @@ -13543,7 +13548,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { } tblProps = validateAndAddDefaultProperties( tblProps, isExt, storageFormat, dbDotTab, sortCols, isMaterialization, isTemporary, - isTransactional, isManaged); + isTransactional, isManaged, false); addDbAndTabToOutputs(new String[] {qualifiedTabName.getDb(), qualifiedTabName.getTable()}, TableType.MANAGED_TABLE, isTemporary, tblProps); @@ -13571,7 +13576,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { qualifiedTabName.getTable() + " cannot be declared transactional because it's an external table"); } tblProps = validateAndAddDefaultProperties(tblProps, isExt, storageFormat, dbDotTab, sortCols, isMaterialization, - isTemporary, isTransactional, isManaged); + isTemporary, isTransactional, isManaged, false); addDbAndTabToOutputs(new String[] {qualifiedTabName.getDb(), qualifiedTabName.getTable()}, TableType.MANAGED_TABLE, false, tblProps); @@ -13595,7 +13600,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { tblProps = validateAndAddDefaultProperties( tblProps, isExt, storageFormat, dbDotTab, sortCols, isMaterialization, isTemporary, - isTransactional, isManaged); + isTransactional, isManaged, false); addDbAndTabToOutputs(new String[] {qualifiedTabName.getDb(), qualifiedTabName.getTable()}, TableType.MANAGED_TABLE, isTemporary, tblProps); @@ -13681,7 +13686,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { tblProps = validateAndAddDefaultProperties( tblProps, isExt, storageFormat, dbDotTab, sortCols, isMaterialization, isTemporary, - isTransactional, isManaged); + isTransactional, isManaged, true); addDbAndTabToOutputs(new String[] {qualifiedTabName.getDb(), qualifiedTabName.getTable()}, TableType.MANAGED_TABLE, isTemporary, tblProps); tableDesc = new CreateTableDesc(qualifiedTabName, isExt, isTemporary, cols, diff --git a/ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java index 55f6149..8318cb9 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hive.ql.plan; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.apache.hive.common.util.HiveStringUtils.quoteComments; import java.io.IOException; @@ -1237,6 +1238,12 @@ public final class PlanUtils { } clone.remove(StatsSetupConst.NUM_ERASURE_CODED_FILES); } + if (properties.containsKey(TABLE_IS_CTAS)) { + if (clone == null) { + clone = new HashMap<>(properties); + } + clone.remove(TABLE_IS_CTAS); + } if (clone != null) { return clone; } diff --git a/standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java b/standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java index 38bb769..ff0c0eb 100644 --- a/standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java +++ b/standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java @@ -61,6 +61,8 @@ package org.apache.hadoop.hive.metastore.api; public static final java.lang.String TABLE_IS_TRANSACTIONAL = "transactional"; + public static final java.lang.String TABLE_IS_CTAS = "created_with_ctas"; + public static final java.lang.String TABLE_NO_AUTO_COMPACT = "no_auto_compaction"; public static final java.lang.String TABLE_TRANSACTIONAL_PROPERTIES = "transactional_properties"; diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java index ec9a76f..4b8d1f0 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java @@ -172,6 +172,7 @@ import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME; import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_COMMENT; import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_NAME; import static org.apache.hadoop.hive.metastore.Warehouse.getCatalogQualifiedTableName; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.CAT_NAME; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.DB_NAME; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog; @@ -2140,6 +2141,9 @@ public class HMSHandler extends FacebookBase implements IHMSHandler { if (transformer != null && !isInTest) { tbl = transformer.transformCreateTable(tbl, processorCapabilities, processorId); } + if (tbl.getParameters() != null) { + tbl.getParameters().remove(TABLE_IS_CTAS); + } // If the given table has column statistics, save it here. We will update it later. // We don't want it to be part of the Table object being created, lest the create table diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java index bbd46d2..df6dc98 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hive.metastore; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; +import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_CTAS; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_IS_TRANSACTIONAL; import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.TABLE_TRANSACTIONAL_PROPERTIES; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.EXTERNAL_TABLE_PURGE; @@ -583,29 +584,34 @@ public class MetastoreDefaultTransformer implements IMetaStoreMetadataTransforme throw new MetaException("Database " + dbName + " for table " + table.getTableName() + " could not be found"); } - if (TableType.MANAGED_TABLE.name().equals(tableType)) { + if (TableType.MANAGED_TABLE.name().equals(tableType)) { LOG.debug("Table is a MANAGED_TABLE"); txnal = params.get(TABLE_IS_TRANSACTIONAL); txn_properties = params.get(TABLE_TRANSACTIONAL_PROPERTIES); + boolean ctas = Boolean.valueOf(params.getOrDefault(TABLE_IS_CTAS, "false")); isInsertAcid = (txn_properties != null && txn_properties.equalsIgnoreCase("insert_only")); if ((txnal == null || txnal.equalsIgnoreCase("FALSE")) && !isInsertAcid) { // non-ACID MANAGED TABLE - LOG.info("Converting " + newTable.getTableName() + " to EXTERNAL tableType for " + processorId); - newTable.setTableType(TableType.EXTERNAL_TABLE.toString()); - params.remove(TABLE_IS_TRANSACTIONAL); - params.remove(TABLE_TRANSACTIONAL_PROPERTIES); - params.put("EXTERNAL", "TRUE"); - params.put(EXTERNAL_TABLE_PURGE, "TRUE"); - params.put("TRANSLATED_TO_EXTERNAL", "TRUE"); - newTable.setParameters(params); - LOG.info("Modified table params are:" + params.toString()); - - if (!table.isSetSd() || table.getSd().getLocation() == null) { - try { - Path newPath = hmsHandler.getWh().getDefaultTablePath(db, table.getTableName(), true); - newTable.getSd().setLocation(newPath.toString()); - LOG.info("Modified location from null to " + newPath); - } catch (Exception e) { - LOG.warn("Exception determining external table location:" + e.getMessage()); + if (ctas) { + LOG.info("Not Converting CTAS table " + newTable.getTableName() + " to EXTERNAL tableType for " + processorId); + } else { + LOG.info("Converting " + newTable.getTableName() + " to EXTERNAL tableType for " + processorId); + newTable.setTableType(TableType.EXTERNAL_TABLE.toString()); + params.remove(TABLE_IS_TRANSACTIONAL); + params.remove(TABLE_TRANSACTIONAL_PROPERTIES); + params.put("EXTERNAL", "TRUE"); + params.put(EXTERNAL_TABLE_PURGE, "TRUE"); + params.put("TRANSLATED_TO_EXTERNAL", "TRUE"); + newTable.setParameters(params); + LOG.info("Modified table params are:" + params.toString()); + + if (!table.isSetSd() || table.getSd().getLocation() == null) { + try { + Path newPath = hmsHandler.getWh().getDefaultTablePath(db, table.getTableName(), true); + newTable.getSd().setLocation(newPath.toString()); + LOG.info("Modified location from null to " + newPath); + } catch (Exception e) { + LOG.warn("Exception determining external table location:" + e.getMessage()); + } } } } else { // ACID table