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

Reply via email to