You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2018/08/16 18:49:43 UTC

hive git commit: HIVE-20378 : don't update stats during alter for txn table conversion (Sergey Shelukhin, reviewed by Jason Dere)

Repository: hive
Updated Branches:
  refs/heads/master 61372dfaf -> a72693cc3


HIVE-20378 : don't update stats during alter for txn table conversion (Sergey Shelukhin, reviewed by Jason Dere)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/a72693cc
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/a72693cc
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/a72693cc

Branch: refs/heads/master
Commit: a72693cc3038e7e5e0f33295645c196ab397dd5c
Parents: 61372df
Author: sergey <se...@apache.org>
Authored: Thu Aug 16 11:49:37 2018 -0700
Committer: sergey <se...@apache.org>
Committed: Thu Aug 16 11:49:37 2018 -0700

----------------------------------------------------------------------
 .../apache/hadoop/hive/metastore/HiveAlterHandler.java   |  4 ++++
 .../org/apache/hadoop/hive/metastore/ObjectStore.java    | 11 +++++------
 .../org/apache/hadoop/hive/metastore/txn/TxnUtils.java   |  1 +
 .../hadoop/hive/metastore/TestHiveAlterHandler.java      | 10 ++++++++--
 4 files changed, 18 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/a72693cc/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index 0441a33..c551b80 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -42,6 +42,7 @@ import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;
 import org.apache.hadoop.hive.metastore.api.Database;
 import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.txn.TxnUtils;
 import org.apache.hadoop.hive.metastore.api.InvalidInputException;
 import org.apache.hadoop.hive.metastore.api.InvalidObjectException;
 import org.apache.hadoop.hive.metastore.api.InvalidOperationException;
@@ -812,6 +813,9 @@ public class HiveAlterHandler implements AlterHandler {
       ColumnStatistics colStats = null;
       boolean updateColumnStats = !newDbName.equals(dbName) || !newTableName.equals(tableName)
           || !MetaStoreServerUtils.columnsIncludedByNameType(oldCols, newCols);
+      // Don't bother in the case of ACID conversion.
+      updateColumnStats = updateColumnStats
+          && (TxnUtils.isAcidTable(oldTable) == TxnUtils.isAcidTable(newTable));
       if (updateColumnStats) {
         List<String> oldColNames = new ArrayList<>(oldCols.size());
         for (FieldSchema oldCol : oldCols) {

http://git-wip-us.apache.org/repos/asf/hive/blob/a72693cc/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
index 8e2f94e..d27224b 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -4112,7 +4112,8 @@ public class ObjectStore implements RawStore, Configurable {
       oldt.setDatabase(newt.getDatabase());
       oldt.setTableName(normalizeIdentifier(newt.getTableName()));
       boolean isTxn = TxnUtils.isTransactionalTable(newTable);
-      if (isTxn && areTxnStatsSupported) {
+      boolean isToTxn = isTxn && !TxnUtils.isTransactionalTable(oldt.getParameters());
+      if (!isToTxn && isTxn && areTxnStatsSupported) {
         // Transactional table is altered without a txn. Make sure there are no changes to the flag.
         String errorMsg = verifyStatsChangeCtx(oldt.getParameters(), newTable.getParameters(),
             newTable.getWriteId(), queryValidWriteIds, false);
@@ -4120,7 +4121,6 @@ public class ObjectStore implements RawStore, Configurable {
           throw new MetaException(errorMsg);
         }
       }
-      boolean isToTxn = isTxn && !TxnUtils.isTransactionalTable(oldt.getParameters());
       oldt.setParameters(newt.getParameters());
       oldt.setOwner(newt.getOwner());
       oldt.setOwnerType(newt.getOwnerType());
@@ -4142,12 +4142,11 @@ public class ObjectStore implements RawStore, Configurable {
       oldt.setRewriteEnabled(newt.isRewriteEnabled());
 
       // If transactional, update the stats state for the current Stats updater query.
-      // Don't update for conversion to acid - it doesn't modify stats but passes in qVWIds.
-      // The fact that it doesn't update stats is verified above.
+      // Set stats invalid for ACID conversion; it doesn't pass in the write ID.
       if (isTxn) {
-        if (!areTxnStatsSupported) {
+        if (!areTxnStatsSupported || isToTxn) {
           StatsSetupConst.setBasicStatsState(oldt.getParameters(), StatsSetupConst.FALSE);
-        } else if (queryValidWriteIds != null && (!isToTxn || newTable.getWriteId() > 0)) {
+        } else if (queryValidWriteIds != null && newTable.getWriteId() > 0) {
           // Check concurrent INSERT case and set false to the flag.
           if (!isCurrentStatsValidForTheQuery(oldt, queryValidWriteIds, true)) {
             StatsSetupConst.setBasicStatsState(oldt.getParameters(), StatsSetupConst.FALSE);

http://git-wip-us.apache.org/repos/asf/hive/blob/a72693cc/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
index aac5811..bd202ed 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
@@ -221,6 +221,7 @@ public class TxnUtils {
       return false;
     }
     Map<String, String> parameters = table.getParameters();
+    if (parameters == null) return false;
     String tableIsTransactional = parameters.get(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL);
     return tableIsTransactional != null && tableIsTransactional.equalsIgnoreCase("true");
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/a72693cc/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java
index 8816480..93b2f23 100644
--- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java
+++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java
@@ -64,7 +64,7 @@ public class TestHiveAlterHandler {
   }
 
   @Test
-  public void testAlterTableDelColUpdateStats() throws MetaException, InvalidObjectException, NoSuchObjectException {
+  public void testAlterTableDelColUpdateStats() throws Exception {
     FieldSchema col1 = new FieldSchema("col1", "string", "col1 comment");
     FieldSchema col2 = new FieldSchema("col2", "string", "col2 comment");
     FieldSchema col3 = new FieldSchema("col3", "string", "col3 comment");
@@ -85,7 +85,13 @@ public class TestHiveAlterHandler {
     RawStore msdb = Mockito.mock(RawStore.class);
     HiveAlterHandler handler = new HiveAlterHandler();
     handler.setConf(conf);
-    handler.alterTableUpdateTableColumnStats(msdb, oldTable, newTable, null, null);
+    try {
+      handler.alterTableUpdateTableColumnStats(msdb, oldTable, newTable, null, null);
+    } catch (Throwable t) {
+      System.err.println(t);
+      t.printStackTrace(System.err);
+      throw t;
+    }
     Mockito.verify(msdb, Mockito.times(1)).getTableColumnStatistics(
         getDefaultCatalog(conf), oldTable.getDbName(), oldTable.getTableName(), Arrays.asList("col1", "col2", "col3", "col4")
     );