hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ekoif...@apache.org
Subject hive git commit: HIVE-11716 Reading ACID table from non-acid session should raise an error (Wei Zheng via Eugene Koifman)
Date Wed, 03 Feb 2016 23:52:18 GMT
Repository: hive
Updated Branches:
  refs/heads/branch-1 556806984 -> caf4b516b


HIVE-11716 Reading ACID table from non-acid session should raise an error (Wei Zheng via Eugene
Koifman)


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

Branch: refs/heads/branch-1
Commit: caf4b516b74201633e25d99bfcf5d69eacc8a617
Parents: 5568069
Author: Eugene Koifman <ekoifman@hortonworks.com>
Authored: Wed Feb 3 15:52:10 2016 -0800
Committer: Eugene Koifman <ekoifman@hortonworks.com>
Committed: Wed Feb 3 15:52:10 2016 -0800

----------------------------------------------------------------------
 .../java/org/apache/hadoop/hive/ql/Driver.java  |  4 ++
 .../org/apache/hadoop/hive/ql/ErrorMsg.java     |  1 +
 .../org/apache/hadoop/hive/ql/io/AcidUtils.java | 40 ++++++++++++++++----
 .../hadoop/hive/ql/lockmgr/DbTxnManager.java    |  5 +++
 .../hadoop/hive/ql/lockmgr/DummyTxnManager.java |  5 +++
 .../hadoop/hive/ql/lockmgr/HiveTxnManager.java  |  6 +++
 .../hadoop/hive/ql/parse/SemanticAnalyzer.java  | 22 ++++++++++-
 .../hadoop/hive/ql/session/SessionState.java    | 10 ++++-
 .../hive/ql/lockmgr/TestDbTxnManager2.java      | 39 ++++++++++++++++++-
 .../clientnegative/delete_not_bucketed.q.out    |  2 +-
 .../clientnegative/update_not_bucketed.q.out    |  2 +-
 11 files changed, 121 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/Driver.java b/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
index 8ccfbfd..fe780ce 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
@@ -520,6 +520,10 @@ public class Driver implements CommandProcessor {
         errorMessage += " " + e.getMessage();
       }
 
+      if (error == ErrorMsg.TXNMGR_NOT_ACID) {
+        errorMessage += ". Failed command: " + queryStr;
+      }
+
       SQLState = error.getSQLState();
       downstreamError = e;
       console.printError(errorMessage, "\n"

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b/ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
index fba1fec..77e82a4 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
@@ -390,6 +390,7 @@ public enum ErrorMsg {
   TXN_ABORTED(10263, "Transaction manager has aborted the transaction {0}.", true),
   DBTXNMGR_REQUIRES_CONCURRENCY(10264,
       "To use DbTxnManager you must set hive.support.concurrency=true"),
+  TXNMGR_NOT_ACID(10265, "This command is not allowed on an ACID table {0}.{1} with a non-ACID
transaction manager", true),
 
   LOCK_NO_SUCH_LOCK(10270, "No record of lock {0} could be found, " +
       "may have timed out", true),

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
index f1ff24c..d98fa93 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
@@ -30,9 +30,10 @@ import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
 import org.apache.hadoop.hive.ql.metadata.Table;
-import org.apache.hadoop.hive.ql.session.SessionState;
 import org.apache.hadoop.hive.shims.HadoopShims;
 import org.apache.hadoop.hive.shims.ShimLoader;
+import org.apache.hadoop.mapred.InputFormat;
+import org.apache.hadoop.mapred.OutputFormat;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -620,17 +621,42 @@ public class AcidUtils {
     HiveConf.setBoolVar(conf, ConfVars.HIVE_TRANSACTIONAL_TABLE_SCAN, isAcidTable);
   }
 
-  // If someone is trying to read a table with transactional=true they must be using the
-  // right TxnManager.  We do not look at SessionState.get().getTxnMgr().supportsAcid().
+  /** Checks metadata to make sure it's a valid ACID table at metadata level
+   * Three things we will check:
+   * 1. TBLPROPERTIES 'transactional'='true'
+   * 2. The table should be bucketed
+   * 3. InputFormatClass/OutputFormatClass should implement AcidInputFormat/AcidOutputFormat
+   *    Currently OrcInputFormat/OrcOutputFormat is the only implementer
+   * Note, users are responsible for using the correct TxnManager. We do not look at
+   * SessionState.get().getTxnMgr().supportsAcid() here
+   * @param table table
+   * @return true if table is a legit ACID table, false otherwise
+   */
   public static boolean isAcidTable(Table table) {
     if (table == null) {
       return false;
     }
-    String tableIsTransactional =
-        table.getProperty(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL);
-    if(tableIsTransactional == null) {
+    String tableIsTransactional = table.getProperty(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL);
+    if (tableIsTransactional == null) {
       tableIsTransactional = table.getProperty(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL.toUpperCase());
     }
-    return tableIsTransactional != null && tableIsTransactional.equalsIgnoreCase("true");
+    if (tableIsTransactional == null || !tableIsTransactional.equalsIgnoreCase("true")) {
+      return false;
+    }
+
+    List<String> bucketCols = table.getBucketCols();
+    if (bucketCols == null || bucketCols.isEmpty()) {
+      return false;
+    }
+
+    Class<? extends InputFormat> inputFormatClass = table.getInputFormatClass();
+    Class<? extends OutputFormat> outputFormatClass = table.getOutputFormatClass();
+    if (inputFormatClass == null || outputFormatClass == null ||
+        !AcidInputFormat.class.isAssignableFrom(inputFormatClass) ||
+        !AcidOutputFormat.class.isAssignableFrom(outputFormatClass)) {
+      return false;
+    }
+
+    return true;
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
index cfdf209..55ea009 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
@@ -423,6 +423,11 @@ public class DbTxnManager extends HiveTxnManagerImpl {
   }
 
   @Override
+  public String getTxnManagerName() {
+    return CLASS_NAME;
+  }
+
+  @Override
   public boolean supportsExplicitLock() {
     return false;
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java
index 4427cf1..3a32a46 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java
@@ -209,6 +209,11 @@ class DummyTxnManager extends HiveTxnManagerImpl {
   }
 
   @Override
+  public String getTxnManagerName() {
+    return DummyTxnManager.class.getName();
+  }
+
+  @Override
   public boolean supportsExplicitLock() {
     return true;
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java
index cb97d29..9b4a97f 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java
@@ -117,6 +117,12 @@ public interface HiveTxnManager {
   ValidTxnList getValidTxns() throws LockException;
 
   /**
+   * Get the name for currently installed transaction manager.
+   * @return transaction manager name
+   */
+  String getTxnManagerName();
+
+  /**
    * This call closes down the transaction manager.  All open transactions
    * are aborted.  If no transactions are open but locks are held those locks
    * are released.  This method should be called if processing of a session

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
----------------------------------------------------------------------
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 dff8ccd..a6c4bdc 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
@@ -1599,8 +1599,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer {
         if ((updating() || deleting()) && !isAcid && isTableWrittenTo) {
           //isTableWrittenTo: delete from acidTbl where a in (select id from nonAcidTable)
           //so only assert this if we are actually writing to this table
-          // isAcidTable above also checks for whether we are using an acid compliant
-          // transaction manager.  But that has already been caught in
+          // Whether we are using an acid compliant transaction manager has already been
caught in
           // UpdateDeleteSemanticAnalyzer, so if we are updating or deleting and getting
nonAcid
           // here, it means the table itself doesn't support it.
           throw new SemanticException(ErrorMsg.ACID_OP_ON_NONACID_TABLE, tab_name);
@@ -10627,6 +10626,13 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer {
                 "Table " + tbl.getTableName() +
                     " Partition " + p.getName()));
       }
+
+      if (p != null) {
+        tbl = p.getTable();
+      }
+      if (tbl != null && AcidUtils.isAcidTable(tbl)) {
+        checkAcidTxnManager(tbl);
+      }
     }
 
     for (WriteEntity writeEntity : getOutputs()) {
@@ -10699,6 +10705,10 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer {
             ErrorMsg.OFFLINE_TABLE_OR_PARTITION.getMsg(
                 "Table " + tbl.getTableName()));
       }
+
+      if (tbl != null && AcidUtils.isAcidTable(tbl)) {
+        checkAcidTxnManager(tbl);
+      }
     }
 
     boolean reworkMapredWork = HiveConf.getBoolVar(this.conf,
@@ -12255,6 +12265,14 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer {
   protected boolean deleting() {
     return false;
   }
+
+  // Make sure the proper transaction manager that supports ACID is being used
+  protected void checkAcidTxnManager(Table table) throws SemanticException {
+    if (SessionState.get() != null && !SessionState.get().getTxnMgr().supportsAcid())
{
+      throw new SemanticException(ErrorMsg.TXNMGR_NOT_ACID, table.getDbName(), table.getTableName());
+    }
+  }
+
   public static ASTNode genSelectDIAST(RowResolver rr) {
     HashMap<String, LinkedHashMap<String, ColumnInfo>> map = rr.getRslvMap();
     ASTNode selectDI = new ASTNode(new CommonToken(HiveParser.TOK_SELECTDI, "TOK_SELECTDI"));

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
index e7dddde..b17942e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
@@ -394,13 +394,19 @@ public class SessionState {
 
   /**
    * Initialize the transaction manager.  This is done lazily to avoid hard wiring one
-   * transaction manager at the beginning of the session.  In general users shouldn't change
-   * this, but it's useful for testing.
+   * transaction manager at the beginning of the session.
    * @param conf Hive configuration to initialize transaction manager
    * @return transaction manager
    * @throws LockException
    */
   public synchronized HiveTxnManager initTxnMgr(HiveConf conf) throws LockException {
+    // Only change txnMgr if the setting has changed
+    if (txnMgr != null &&
+        !txnMgr.getTxnManagerName().equals(conf.getVar(HiveConf.ConfVars.HIVE_TXN_MANAGER)))
{
+      txnMgr.closeTxnManager();
+      txnMgr = null;
+    }
+
     if (txnMgr == null) {
       txnMgr = TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
     }

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
----------------------------------------------------------------------
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
index a559a36..685f42a 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
@@ -27,7 +27,6 @@ import org.apache.hadoop.hive.metastore.txn.TxnDbUtil;
 import org.apache.hadoop.hive.ql.Context;
 import org.apache.hadoop.hive.ql.Driver;
 import org.apache.hadoop.hive.ql.ErrorMsg;
-import org.apache.hadoop.hive.ql.metadata.Hive;
 import org.apache.hadoop.hive.ql.processors.CommandProcessorResponse;
 import org.apache.hadoop.hive.ql.session.SessionState;
 import org.junit.After;
@@ -241,7 +240,43 @@ public class TestDbTxnManager2 {
     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T9", null, locks.get(0));
     otherTxnMgr.closeTxnManager();
   }
-  
+
+  @Test
+  public void testDummyTxnManagerOnAcidTable() throws Exception {
+    // Create an ACID table with DbTxnManager
+    CommandProcessorResponse cpr = driver.run("create table T10 (a int, b int) clustered
by(b) into 2 buckets stored as orc TBLPROPERTIES ('transactional'='true')");
+    checkCmdOnDriver(cpr);
+    cpr = driver.run("create table T11 (a int, b int) clustered by(b) into 2 buckets stored
as orc");
+    checkCmdOnDriver(cpr);
+
+    // Now switch to DummyTxnManager
+    conf.setVar(HiveConf.ConfVars.HIVE_TXN_MANAGER, "org.apache.hadoop.hive.ql.lockmgr.DummyTxnManager");
+    txnMgr = SessionState.get().initTxnMgr(conf);
+    Assert.assertTrue(txnMgr instanceof DummyTxnManager);
+
+    // All DML should fail with DummyTxnManager on ACID table
+    cpr = driver.compileAndRespond("select * from T10");
+    Assert.assertEquals(ErrorMsg.TXNMGR_NOT_ACID.getErrorCode(), cpr.getResponseCode());
+    Assert.assertTrue(cpr.getErrorMessage().contains("This command is not allowed on an ACID
table"));
+
+    cpr = driver.compileAndRespond("insert into table T10 values (1, 2)");
+    Assert.assertEquals(ErrorMsg.TXNMGR_NOT_ACID.getErrorCode(), cpr.getResponseCode());
+    Assert.assertTrue(cpr.getErrorMessage().contains("This command is not allowed on an ACID
table"));
+
+    cpr = driver.compileAndRespond("insert overwrite table T10 select a, b from T11");
+    Assert.assertEquals(ErrorMsg.NO_INSERT_OVERWRITE_WITH_ACID.getErrorCode(), cpr.getResponseCode());
+    Assert.assertTrue(cpr.getErrorMessage().contains("INSERT OVERWRITE not allowed on table
with OutputFormat" +
+        " that implements AcidOutputFormat while transaction manager that supports ACID is
in use"));
+
+    cpr = driver.compileAndRespond("update T10 set a=0 where b=1");
+    Assert.assertEquals(ErrorMsg.ACID_OP_ON_NONACID_TXNMGR.getErrorCode(), cpr.getResponseCode());
+    Assert.assertTrue(cpr.getErrorMessage().contains("Attempt to do update or delete using
transaction manager that does not support these operations."));
+
+    cpr = driver.compileAndRespond("delete from T10");
+    Assert.assertEquals(ErrorMsg.ACID_OP_ON_NONACID_TXNMGR.getErrorCode(), cpr.getResponseCode());
+    Assert.assertTrue(cpr.getErrorMessage().contains("Attempt to do update or delete using
transaction manager that does not support these operations."));
+  }
+
   private void checkLock(LockType type, LockState state, String db, String table, String
partition, ShowLocksResponseElement l) {
     Assert.assertEquals(l.toString(),l.getType(), type);
     Assert.assertEquals(l.toString(),l.getState(), state);

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/test/results/clientnegative/delete_not_bucketed.q.out
----------------------------------------------------------------------
diff --git a/ql/src/test/results/clientnegative/delete_not_bucketed.q.out b/ql/src/test/results/clientnegative/delete_not_bucketed.q.out
index d0ba680..8c4a40c 100644
--- a/ql/src/test/results/clientnegative/delete_not_bucketed.q.out
+++ b/ql/src/test/results/clientnegative/delete_not_bucketed.q.out
@@ -6,4 +6,4 @@ POSTHOOK: query: create table acid_notbucketed(a int, b varchar(128)) stored
as
 POSTHOOK: type: CREATETABLE
 POSTHOOK: Output: database:default
 POSTHOOK: Output: default@acid_notbucketed
-FAILED: SemanticException [Error 10297]: Attempt to do update or delete on table acid_notbucketed
that does not use an AcidOutputFormat or is not bucketed
+FAILED: SemanticException [Error 10297]: Attempt to do update or delete on table default.acid_notbucketed
that does not use an AcidOutputFormat or is not bucketed

http://git-wip-us.apache.org/repos/asf/hive/blob/caf4b516/ql/src/test/results/clientnegative/update_not_bucketed.q.out
----------------------------------------------------------------------
diff --git a/ql/src/test/results/clientnegative/update_not_bucketed.q.out b/ql/src/test/results/clientnegative/update_not_bucketed.q.out
index 8ebf41d..42a48a0 100644
--- a/ql/src/test/results/clientnegative/update_not_bucketed.q.out
+++ b/ql/src/test/results/clientnegative/update_not_bucketed.q.out
@@ -6,4 +6,4 @@ POSTHOOK: query: create table acid_notbucketed(a int, b varchar(128)) partitione
 POSTHOOK: type: CREATETABLE
 POSTHOOK: Output: database:default
 POSTHOOK: Output: default@acid_notbucketed
-FAILED: SemanticException [Error 10297]: Attempt to do update or delete on table acid_notbucketed
that does not use an AcidOutputFormat or is not bucketed
+FAILED: SemanticException [Error 10297]: Attempt to do update or delete on table default.acid_notbucketed
that does not use an AcidOutputFormat or is not bucketed


Mime
View raw message