You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by vi...@apache.org on 2017/11/14 00:00:23 UTC
hive git commit: HIVE-17942 : HiveAlterHandler not using conf from
HMS Handler (Janaki Lahorani, reviewed by Vihang Karajgaonkar)
Repository: hive
Updated Branches:
refs/heads/master 83971dec5 -> 4d32b8fc3
HIVE-17942 : HiveAlterHandler not using conf from HMS Handler (Janaki Lahorani, reviewed by Vihang Karajgaonkar)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/4d32b8fc
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/4d32b8fc
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/4d32b8fc
Branch: refs/heads/master
Commit: 4d32b8fc3e12b91434eaa816fef17a09e69cf414
Parents: 83971de
Author: Vihang Karajgaonkar <vi...@cloudera.com>
Authored: Mon Nov 13 15:41:50 2017 -0800
Committer: Vihang Karajgaonkar <vi...@cloudera.com>
Committed: Mon Nov 13 15:41:50 2017 -0800
----------------------------------------------------------------------
.../TestHiveMetaStoreAlterColumnPar.java | 115 +++++++++++++++++++
.../hadoop/hive/metastore/HiveAlterHandler.java | 15 ++-
2 files changed, 124 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/4d32b8fc/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java
new file mode 100644
index 0000000..569e932
--- /dev/null
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.Statement;
+import java.util.HashMap;
+
+/**
+ * Test that set/unset of metaconf:hive.metastore.disallow.incompatible.col.type.changes is local
+ * to the session
+ *
+ * Two connections are created. In one connection
+ * metaconf:hive.metastore.disallow.incompatible.col.type.changes is set to false. Then the
+ * columm type can changed from int to smallint. In the other connection
+ * metaconf:hive.metastore.disallow.incompatible.col.type.changes is left with the default value
+ * true. In that connection changing column type from int to smallint will throw an error.
+ *
+ */
+public class TestHiveMetaStoreAlterColumnPar {
+
+ public static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void startServices() throws Exception {
+ HiveConf hiveConf = new HiveConf();
+ hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_MIN_WORKER_THREADS, 2);
+ hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_MAX_WORKER_THREADS, 2);
+ hiveConf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+
+ miniHS2 = new MiniHS2.Builder().withMiniMR().withRemoteMetastore().withConf(hiveConf).build();
+
+ miniHS2.start(new HashMap<String, String>());
+
+ Connection hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL(), "hive", "hive");
+ Statement stmt = hs2Conn.createStatement();
+
+ // create table
+ stmt.execute("drop table if exists t1");
+ stmt.execute("create table t1 (c1 int)");
+
+ stmt.close();
+ hs2Conn.close();
+ }
+
+ @AfterClass
+ public static void stopServices() throws Exception {
+ if (miniHS2 != null && miniHS2.isStarted()) {
+ miniHS2.stop();
+ }
+ }
+
+ @Test
+ public void testAlterColumn() throws Exception {
+ assertTrue("Test setup failed. MiniHS2 is not initialized",
+ miniHS2 != null && miniHS2.isStarted());
+
+ try (
+ Connection hs2Conn1 = DriverManager
+ .getConnection(TestHiveMetaStoreAlterColumnPar.miniHS2.getJdbcURL(), "hive", "hive");
+ Statement stmt1 = hs2Conn1.createStatement();
+ Connection hs2Conn2 = DriverManager
+ .getConnection(TestHiveMetaStoreAlterColumnPar.miniHS2.getJdbcURL(), "hive", "hive");
+ Statement stmt2 = hs2Conn2.createStatement();
+ ) {
+ // Set parameter to be false in connection 1. int to smallint allowed
+ stmt1.execute("set metaconf:hive.metastore.disallow.incompatible.col.type.changes=false");
+ stmt1.execute("alter table t1 change column c1 c1 smallint");
+
+ // Change the type back to int so that the same alter can be attempted from connection 2.
+ stmt1.execute("alter table t1 change column c1 c1 int");
+
+ // parameter value not changed to false in connection 2. int to smallint throws exception
+ try {
+ stmt2.execute("alter table t1 change column c1 c1 smallint");
+ assertTrue("Exception not thrown", true);
+ } catch (Exception e1) {
+ assertTrue("Unexpected exception: " + e1.getMessage(), e1.getMessage().contains(
+ "Unable to alter table. The following columns have types incompatible with the existing columns in their respective positions"));
+ }
+
+ // parameter value is still false in 1st connection. The alter still goes through.
+ stmt1.execute("alter table t1 change column c1 c1 smallint");
+ } catch (Exception e2) {
+ assertTrue("Unexpected Exception: " + e2.getMessage(), true);
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/hive/blob/4d32b8fc/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index 93de719..b445723 100644
--- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -71,6 +71,9 @@ public class HiveAlterHandler implements AlterHandler {
private static final Logger LOG = LoggerFactory.getLogger(HiveAlterHandler.class
.getName());
+ // hiveConf, getConf and setConf are in this class because AlterHandler extends Configurable.
+ // Always use the configuration from HMS Handler. Making AlterHandler not extend Configurable
+ // is not in the scope of the fix for HIVE-17942.
@Override
public Configuration getConf() {
return hiveConf;
@@ -107,7 +110,7 @@ public class HiveAlterHandler implements AlterHandler {
String newTblName = newt.getTableName().toLowerCase();
String newDbName = newt.getDbName().toLowerCase();
- if (!MetaStoreUtils.validateName(newTblName, hiveConf)) {
+ if (!MetaStoreUtils.validateName(newTblName, handler.getConf())) {
throw new InvalidOperationException(newTblName + " is not a valid object name");
}
String validate = MetaStoreUtils.validateTblColumns(newt.getSd().getCols());
@@ -156,7 +159,7 @@ public class HiveAlterHandler implements AlterHandler {
// Views derive the column type from the base table definition. So the view definition
// can be altered to change the column types. The column type compatibility checks should
// be done only for non-views.
- if (MetastoreConf.getBoolVar(hiveConf,
+ if (MetastoreConf.getBoolVar(handler.getConf(),
MetastoreConf.ConfVars.DISALLOW_INCOMPATIBLE_COL_TYPE_CHANGES) &&
!oldt.getTableType().equals(TableType.VIRTUAL_VIEW.toString())) {
// Throws InvalidOperationException if the new column types are not
@@ -290,7 +293,7 @@ public class HiveAlterHandler implements AlterHandler {
}
} else {
// operations other than table rename
- if (MetaStoreUtils.requireCalStats(hiveConf, null, null, newt, environmentContext) &&
+ if (MetaStoreUtils.requireCalStats(handler.getConf(), null, null, newt, environmentContext) &&
!isPartitionedTable) {
Database db = msdb.getDatabase(newDbName);
// Update table stats. For partitioned table, we update stats in alterPartition()
@@ -436,7 +439,7 @@ public class HiveAlterHandler implements AlterHandler {
try {
msdb.openTransaction();
oldPart = msdb.getPartition(dbname, name, new_part.getValues());
- if (MetaStoreUtils.requireCalStats(hiveConf, oldPart, new_part, tbl, environmentContext)) {
+ if (MetaStoreUtils.requireCalStats(handler.getConf(), oldPart, new_part, tbl, environmentContext)) {
// if stats are same, no need to update
if (MetaStoreUtils.isFastStatsSame(oldPart, new_part)) {
MetaStoreUtils.updateBasicState(environmentContext, new_part.getParameters());
@@ -569,7 +572,7 @@ public class HiveAlterHandler implements AlterHandler {
new_part.getSd().setLocation(oldPart.getSd().getLocation());
}
- if (MetaStoreUtils.requireCalStats(hiveConf, oldPart, new_part, tbl, environmentContext)) {
+ if (MetaStoreUtils.requireCalStats(handler.getConf(), oldPart, new_part, tbl, environmentContext)) {
MetaStoreUtils.updatePartitionStatsFast(new_part, wh, false, true, environmentContext);
}
@@ -661,7 +664,7 @@ public class HiveAlterHandler implements AlterHandler {
oldParts.add(oldTmpPart);
partValsList.add(tmpPart.getValues());
- if (MetaStoreUtils.requireCalStats(hiveConf, oldTmpPart, tmpPart, tbl, environmentContext)) {
+ if (MetaStoreUtils.requireCalStats(handler.getConf(), oldTmpPart, tmpPart, tbl, environmentContext)) {
// Check if stats are same, no need to update
if (MetaStoreUtils.isFastStatsSame(oldTmpPart, tmpPart)) {
MetaStoreUtils.updateBasicState(environmentContext, tmpPart.getParameters());