You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by pv...@apache.org on 2018/03/20 09:04:29 UTC

hive git commit: HIVE-18892: Fix NPEs in HiveMetastore.exchange_partitions method (Marta Kuczora, reviewed by Sahil Takiar and Peter Vary)

Repository: hive
Updated Branches:
  refs/heads/master 68459cf0b -> 0870ab9ca


HIVE-18892: Fix NPEs in HiveMetastore.exchange_partitions method (Marta Kuczora, reviewed by Sahil Takiar and Peter Vary)


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

Branch: refs/heads/master
Commit: 0870ab9ca6b622b850d83c0a583e1c3a123c33e7
Parents: 68459cf
Author: Peter Vary <pv...@cloudera.com>
Authored: Tue Mar 20 10:03:37 2018 +0100
Committer: Peter Vary <pv...@cloudera.com>
Committed: Tue Mar 20 10:03:37 2018 +0100

----------------------------------------------------------------------
 .../hadoop/hive/metastore/HiveMetaStore.java    |  13 +
 .../client/TestExchangePartitions.java          | 288 +++++--------------
 2 files changed, 92 insertions(+), 209 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/0870ab9c/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 5285570..6838dd7 100644
--- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -3389,12 +3389,25 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     public List<Partition> exchange_partitions(Map<String, String> partitionSpecs,
         String sourceDbName, String sourceTableName, String destDbName,
         String destTableName) throws TException {
+      if (partitionSpecs == null || sourceDbName == null || sourceTableName == null
+          || destDbName == null || destTableName == null) {
+        throw new MetaException("The DB and table name for the source and destination tables,"
+            + " and the partition specs must not be null.");
+      }
       boolean success = false;
       boolean pathCreated = false;
       RawStore ms = getMS();
       ms.openTransaction();
       Table destinationTable = ms.getTable(destDbName, destTableName);
+      if (destinationTable == null) {
+        throw new MetaException(
+            "The destination table " + destDbName + "." + destTableName + " not found");
+      }
       Table sourceTable = ms.getTable(sourceDbName, sourceTableName);
+      if (sourceTable == null) {
+        throw new MetaException(
+            "The source table " + sourceDbName + "." + sourceTableName + " not found");
+      }
       List<String> partVals = MetaStoreUtils.getPvals(sourceTable.getPartitionKeys(),
           partitionSpecs);
       List<String> partValsPresent = new ArrayList<> ();

http://git-wip-us.apache.org/repos/asf/hive/blob/0870ab9c/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestExchangePartitions.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestExchangePartitions.java b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestExchangePartitions.java
index 5b21491..c9b9e9b 100644
--- a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestExchangePartitions.java
+++ b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestExchangePartitions.java
@@ -38,7 +38,6 @@ import org.apache.hadoop.hive.metastore.client.builder.PartitionBuilder;
 import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
 import org.apache.hadoop.hive.metastore.minihms.AbstractMetaStoreService;
 import org.apache.thrift.TException;
-import org.apache.thrift.transport.TTransportException;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -290,160 +289,100 @@ public class TestExchangePartitions extends MetaStoreClientTest {
         sourceTable.getTableName(), destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNonExistingSourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, DB_NAME, "nonexistingtable",
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, DB_NAME, "nonexistingtable", destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNonExistingSourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, "nonexistingdb", sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, "nonexistingdb", sourceTable.getTableName(),
+        destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNonExistingDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, "nonexistingtable");
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, "nonexistingtable");
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNonExistingDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), "nonexistingdb", destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        "nonexistingdb", destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsEmptySourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, DB_NAME, "", destTable.getDbName(),
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, DB_NAME, "", destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsEmptySourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, "", sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, "", sourceTable.getTableName(),
+        destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsEmptyDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, "");
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, "");
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsEmptyDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), "", destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        "", destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNullSourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, DB_NAME, null, destTable.getDbName(),
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, DB_NAME, null, destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNullSourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, null, sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, null, sourceTable.getTableName(),
+        destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNullDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, null);
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, null);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNullDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partitions(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), null, destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partitions(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        null, destTable.getTableName());
   }
 
   @Test(expected = MetaException.class)
@@ -454,15 +393,10 @@ public class TestExchangePartitions extends MetaStoreClientTest {
         sourceTable.getTableName(), destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionsNullPartSpec() throws Exception {
-    try {
-      client.exchange_partitions(null, sourceTable.getDbName(), sourceTable.getTableName(), null,
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: NPE should not be thrown
-    }
+    client.exchange_partitions(null, sourceTable.getDbName(), sourceTable.getTableName(), null,
+        destTable.getTableName());
   }
 
   @Test(expected = MetaException.class)
@@ -881,160 +815,100 @@ public class TestExchangePartitions extends MetaStoreClientTest {
         sourceTable.getTableName(), destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNonExistingSourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, DB_NAME, "nonexistingtable",
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, DB_NAME, "nonexistingtable", destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNonExistingSourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, "nonexistingdb", sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, "nonexistingdb", sourceTable.getTableName(),
+        destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNonExistingDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, "nonexistingtable");
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, "nonexistingtable");
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNonExistingDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), "nonexistingdb", destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        "nonexistingdb", destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionEmptySourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, DB_NAME, "", destTable.getDbName(),
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, DB_NAME, "", destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionEmptySourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, "", sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, "", sourceTable.getTableName(), destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionEmptyDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, "");
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, "");
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionEmptyDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), "", destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        "", destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNullSourceTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, DB_NAME, null, destTable.getDbName(),
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, DB_NAME, null, destTable.getDbName(),
+        destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNullSourceDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, null, sourceTable.getTableName(),
-          destTable.getDbName(), destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, null, sourceTable.getTableName(),
+        destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNullDestTable() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), DB_NAME, null);
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        DB_NAME, null);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNullDestDB() throws Exception {
 
     Map<String, String> partitionSpecs = getPartitionSpec(partitions[1]);
-    try {
-      client.exchange_partition(partitionSpecs, sourceTable.getDbName(),
-          sourceTable.getTableName(), null, destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: Non existing table or db should be handled correctly and NPE should not occur.
-    }
+    client.exchange_partition(partitionSpecs, sourceTable.getDbName(), sourceTable.getTableName(),
+        null, destTable.getTableName());
   }
 
   @Test(expected = MetaException.class)
@@ -1045,15 +919,11 @@ public class TestExchangePartitions extends MetaStoreClientTest {
         sourceTable.getTableName(), destTable.getDbName(), destTable.getTableName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testExchangePartitionNullPartSpec() throws Exception {
-    try {
-      client.exchange_partition(null, sourceTable.getDbName(), sourceTable.getTableName(), null,
-          destTable.getTableName());
-      Assert.fail("Exception should have been thrown.");
-    } catch (TTransportException | NullPointerException e) {
-      // TODO: NPE should not be thrown
-    }
+
+    client.exchange_partition(null, sourceTable.getDbName(), sourceTable.getTableName(), null,
+        destTable.getTableName());
   }
 
   @Test(expected = MetaException.class)