You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by sa...@apache.org on 2019/04/16 10:13:35 UTC

[hive] branch master updated: HIVE-21500: Disable conversion of managed table to external and vice versa at source via alter table (Sankar Hariappan, reviewed by Ashutosh Bapat, Anishek Agarwal)

This is an automated email from the ASF dual-hosted git repository.

sankarh 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 d519c9c  HIVE-21500: Disable conversion of managed table to external and vice versa at source via alter table (Sankar Hariappan, reviewed by Ashutosh Bapat, Anishek Agarwal)
d519c9c is described below

commit d519c9c4cba4a1f8dae5ffc2bf685464f1fe9a24
Author: Sankar Hariappan <sa...@apache.org>
AuthorDate: Tue Apr 16 15:43:01 2019 +0530

    HIVE-21500: Disable conversion of managed table to external and vice versa at source via alter table (Sankar Hariappan, reviewed by Ashutosh Bapat, Anishek Agarwal)
    
    Signed-off-by: Sankar Hariappan <sa...@apache.org>
---
 .../parse/TestReplScenariosWithStrictManaged.java  | 71 ++++++++++++++++++++++
 .../TestReplicationScenariosExternalTables.java    | 54 ----------------
 .../parse/TestReplicationWithTableMigration.java   | 28 +++++++++
 .../hadoop/hive/metastore/HiveAlterHandler.java    | 20 ++++++
 .../metastore/TransactionalValidationListener.java |  6 ++
 .../metastore/utils/HiveStrictManagedUtils.java    |  4 ++
 6 files changed, 129 insertions(+), 54 deletions(-)

diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplScenariosWithStrictManaged.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplScenariosWithStrictManaged.java
new file mode 100644
index 0000000..db042e2
--- /dev/null
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplScenariosWithStrictManaged.java
@@ -0,0 +1,71 @@
+/*
+ * 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.ql.parse;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * TestReplScenariosWithStrictManaged - Test all replication scenarios with strict managed enabled
+ * at source and target.
+ */
+public class TestReplScenariosWithStrictManaged extends BaseReplicationAcrossInstances {
+
+  @BeforeClass
+  public static void classLevelSetup() throws Exception {
+    Map<String, String> overrides = new HashMap<>();
+    overrides.put(HiveConf.ConfVars.HIVE_STRICT_MANAGED_TABLES.varname, "true");
+    overrides.put(MetastoreConf.ConfVars.CREATE_TABLES_AS_ACID.getHiveName(), "true");
+    overrides.put(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY.varname, "true");
+    overrides.put(HiveConf.ConfVars.HIVE_TXN_MANAGER.varname, "org.apache.hadoop.hive.ql.lockmgr.DbTxnManager");
+
+    internalBeforeClassSetup(overrides, TestReplScenariosWithStrictManaged.class);
+  }
+
+  @Test
+  public void dynamicallyConvertManagedToExternalTable() throws Throwable {
+    // All tables are automatically converted to ACID tables when strict managed is enabled.
+    // Also, it is not possible to convert ACID table to external table.
+    primary.run("use " + primaryDbName)
+            .run("create table t1 (id int) stored as orc")
+            .run("insert into table t1 values (1)")
+            .run("create table t2 (id int) partitioned by (key int) stored as orc")
+            .run("insert into table t2 partition(key=10) values (1)")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='true')")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='true', 'TRANSACTIONAL'='false')")
+            .runFailure("alter table t2 set tblproperties('EXTERNAL'='true')");
+  }
+
+  @Test
+  public void dynamicallyConvertExternalToManagedTable() throws Throwable {
+    // With Strict managed enabled, it is not possible to convert external table to ACID table.
+    primary.run("use " + primaryDbName)
+            .run("create external table t1 (id int) stored as orc")
+            .run("insert into table t1 values (1)")
+            .run("create external table t2 (place string) partitioned by (country string)")
+            .run("insert into table t2 partition(country='india') values ('bangalore')")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='false')")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='false', 'TRANSACTIONAL'='true')")
+            .runFailure("alter table t2 set tblproperties('EXTERNAL'='false')");
+  }
+}
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
index b8e96f2..72da2f1 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
@@ -25,7 +25,6 @@ import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.InjectableBehaviourObjectStore;
 import org.apache.hadoop.hive.metastore.InjectableBehaviourObjectStore.BehaviourInjection;
 import org.apache.hadoop.hive.metastore.InjectableBehaviourObjectStore.CallerArguments;
-import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
 import org.apache.hadoop.hive.metastore.messaging.json.gzip.GzipJSONMessageEncoder;
 import org.apache.hadoop.hive.ql.ErrorMsg;
@@ -674,59 +673,6 @@ public class TestReplicationScenariosExternalTables extends BaseReplicationAcros
             ErrorMsg.REPL_BOOTSTRAP_LOAD_PATH_NOT_VALID.getErrorCode());
   }
 
-  @Test
-  public void dynamicallyConvertManagedToExternalTable() throws Throwable {
-    List<String> dumpWithClause = Collections.singletonList(
-            "'" + HiveConf.ConfVars.REPL_INCLUDE_EXTERNAL_TABLES.varname + "'='true'"
-    );
-    List<String> loadWithClause = externalTableBasePathWithClause();
-
-    WarehouseInstance.Tuple tupleBootstrapManagedTable = primary.run("use " + primaryDbName)
-            .run("create table t1 (id int)")
-            .run("insert into table t1 values (1)")
-            .run("create table t2 (id int) partitioned by (key int)")
-            .run("insert into table t2 partition(key=10) values (1)")
-            .dump(primaryDbName, null, dumpWithClause);
-
-    replica.load(replicatedDbName, tupleBootstrapManagedTable.dumpLocation, loadWithClause);
-
-    Hive hiveForReplica = Hive.get(replica.hiveConf);
-    Table replicaTable = hiveForReplica.getTable(replicatedDbName + ".t1");
-    Path oldTblLocT1 = replicaTable.getDataLocation();
-
-    replicaTable = hiveForReplica.getTable(replicatedDbName + ".t2");
-    Path oldTblLocT2 = replicaTable.getDataLocation();
-
-    WarehouseInstance.Tuple tupleIncConvertToExternalTbl = primary.run("use " + primaryDbName)
-            .run("alter table t1 set tblproperties('EXTERNAL'='true')")
-            .run("alter table t2 set tblproperties('EXTERNAL'='true')")
-            .dump(primaryDbName, tupleBootstrapManagedTable.lastReplicationId, dumpWithClause);
-
-    assertExternalFileInfo(Arrays.asList("t1", "t2"),
-            new Path(tupleIncConvertToExternalTbl.dumpLocation, FILE_NAME));
-    replica.load(replicatedDbName, tupleIncConvertToExternalTbl.dumpLocation, loadWithClause)
-            .run("use " + replicatedDbName)
-            .run("select id from t1")
-            .verifyResult("1")
-            .run("select id from t2 where key=10")
-            .verifyResult("1");
-
-    // Check if the table type is set correctly in target.
-    replicaTable = hiveForReplica.getTable(replicatedDbName + ".t1");
-    assertTrue(TableType.EXTERNAL_TABLE.equals(replicaTable.getTableType()));
-
-    replicaTable = hiveForReplica.getTable(replicatedDbName + ".t2");
-    assertTrue(TableType.EXTERNAL_TABLE.equals(replicaTable.getTableType()));
-
-    // Verify if new table location is set inside the base directory.
-    assertTablePartitionLocation(primaryDbName + ".t1", replicatedDbName + ".t1");
-    assertTablePartitionLocation(primaryDbName + ".t2", replicatedDbName + ".t2");
-
-    // Old location should be removed and set to new location.
-    assertFalse(replica.miniDFSCluster.getFileSystem().exists(oldTblLocT1));
-    assertFalse(replica.miniDFSCluster.getFileSystem().exists(oldTblLocT2));
-  }
-
   private List<String> externalTableBasePathWithClause() throws IOException, SemanticException {
     Path externalTableLocation = new Path(REPLICA_EXTERNAL_BASE);
     DistributedFileSystem fileSystem = replica.miniDFSCluster.getFileSystem();
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java
index bafcdbe..41f2b9d 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java
@@ -476,4 +476,32 @@ public class TestReplicationWithTableMigration {
     replica.load(replicatedDbName, tuple.dumpLocation, withConfigs);
     verifyLoadExecution(replicatedDbName, tuple.lastReplicationId);
   }
+
+  @Test
+  public void dynamicallyConvertManagedToExternalTable() throws Throwable {
+    // With Strict managed disabled but Db enabled for replication, it is not possible to convert
+    // external table to managed table.
+    primary.run("use " + primaryDbName)
+            .run("create table t1 (id int) clustered by(id) into 3 buckets stored as orc ")
+            .run("insert into t1 values(1)")
+            .run("create table t2 partitioned by (country string) ROW FORMAT SERDE "
+                    + "'org.apache.hadoop.hive.serde2.avro.AvroSerDe' stored as avro "
+                    + "tblproperties ('avro.schema.url'='" + avroSchemaFile.toUri().toString() + "')")
+            .run("insert into t2 partition (country='india') values ('another', 13)")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='true')")
+            .runFailure("alter table t2 set tblproperties('EXTERNAL'='true')");
+  }
+
+  @Test
+  public void dynamicallyConvertExternalToManagedTable() throws Throwable {
+    // With Strict managed disabled but Db enabled for replication, it is not possible to convert
+    // external table to managed table.
+    primary.run("use " + primaryDbName)
+            .run("create external table t1 (id int) stored as orc")
+            .run("insert into table t1 values (1)")
+            .run("create external table t2 (place string) partitioned by (country string)")
+            .run("insert into table t2 partition(country='india') values ('bangalore')")
+            .runFailure("alter table t1 set tblproperties('EXTERNAL'='false')")
+            .runFailure("alter table t2 set tblproperties('EXTERNAL'='false')");
+  }
 }
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 ad670c9..d4aaa5c 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
@@ -169,6 +169,8 @@ public class HiveAlterHandler implements AlterHandler {
             TableName.getQualified(catName, dbname, name) + " doesn't exist");
       }
 
+      checkTableTypeConversion(olddb, oldt, newt);
+
       if (oldt.getPartitionKeysSize() != 0) {
         isPartitionedTable = true;
       }
@@ -803,6 +805,24 @@ public class HiveAlterHandler implements AlterHandler {
     return oldParts;
   }
 
+  private void checkTableTypeConversion(Database db, Table oldTbl, Table newTbl)
+          throws InvalidOperationException {
+    // If the given DB is enabled for replication and strict managed is false, then table type cannot be changed.
+    // This is to avoid migration scenarios which causes Managed ACID table to be converted to external at replica.
+    // As ACID tables cannot be converted to external table and vice versa, we need to restrict this conversion at
+    // primary as well.
+    // Currently, table type conversion is allowed only between managed and external table types.
+    // But, to be future proof, any table type conversion is restricted on a replication enabled DB.
+    if (!conf.getBoolean(MetastoreConf.ConfVars.STRICT_MANAGED_TABLES.getHiveName(), false)
+        && !oldTbl.getTableType().equalsIgnoreCase(newTbl.getTableType())
+        && ReplChangeManager.isSourceOfReplication(db)) {
+      throw new InvalidOperationException("Table type cannot be changed from " + oldTbl.getTableType()
+              + " to " + newTbl.getTableType() + " for the table " +
+              TableName.getQualified(oldTbl.getCatName(), oldTbl.getDbName(), oldTbl.getTableName())
+              + " as it is enabled for replication.");
+    }
+  }
+
   private boolean checkPartialPartKeysEqual(List<FieldSchema> oldPartKeys,
       List<FieldSchema> newPartKeys) {
     //return true if both are null, or false if one is null and the other isn't
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
index 004acf8..afa6e4c 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
@@ -146,6 +146,12 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
     }
 
     if (transactionalValuePresent) {
+      if (oldTable.getTableType().equals(TableType.MANAGED_TABLE.toString())
+              && newTable.getTableType().equals(TableType.EXTERNAL_TABLE.toString())) {
+        throw new MetaException(Warehouse.getQualifiedName(newTable) +
+                " cannot be converted to external table as it is transactional table.");
+      }
+
       //normalize prop name
       parameters.put(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL, transactionalValue);
     }
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/HiveStrictManagedUtils.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/HiveStrictManagedUtils.java
index 7213f8e..4610cdd 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/HiveStrictManagedUtils.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/HiveStrictManagedUtils.java
@@ -59,6 +59,10 @@ public class HiveStrictManagedUtils {
         if (isAvroTableWithExternalSchema(table)) {
           return createValidationError(table, "Managed Avro table has externally defined schema.");
         }
+      } else if (tableType == TableType.EXTERNAL_TABLE) {
+        if (MetaStoreServerUtils.isTransactionalTable(table.getParameters())) {
+          return createValidationError(table, "Table is marked as a external table but it is transactional.");
+        }
       }
     }