Return-Path: X-Original-To: apmail-hive-commits-archive@www.apache.org Delivered-To: apmail-hive-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 51E7B188B6 for ; Wed, 3 Feb 2016 23:52:18 +0000 (UTC) Received: (qmail 40672 invoked by uid 500); 3 Feb 2016 23:52:18 -0000 Delivered-To: apmail-hive-commits-archive@hive.apache.org Received: (qmail 40624 invoked by uid 500); 3 Feb 2016 23:52:18 -0000 Mailing-List: contact commits-help@hive.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: hive-dev@hive.apache.org Delivered-To: mailing list commits@hive.apache.org Received: (qmail 40613 invoked by uid 99); 3 Feb 2016 23:52:18 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 03 Feb 2016 23:52:18 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 129A7DFC8F; Wed, 3 Feb 2016 23:52:18 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: ekoifman@apache.org To: commits@hive.apache.org Message-Id: <5a4028272cda4a1095b2dddfad5a4ce5@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: hive git commit: HIVE-11716 Reading ACID table from non-acid session should raise an error (Wei Zheng via Eugene Koifman) Date: Wed, 3 Feb 2016 23:52:18 +0000 (UTC) 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 Authored: Wed Feb 3 15:52:10 2016 -0800 Committer: Eugene Koifman 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 bucketCols = table.getBucketCols(); + if (bucketCols == null || bucketCols.isEmpty()) { + return false; + } + + Class inputFormatClass = table.getInputFormatClass(); + Class 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> 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