hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pv...@apache.org
Subject hive git commit: HIVE-19076: Fix NPE and TApplicationException in function related HiveMetastore methods (Marta Kuczora, via Peter Vary)
Date Tue, 10 Apr 2018 13:49:18 GMT
Repository: hive
Updated Branches:
  refs/heads/master 5eed779c6 -> 820db608f


HIVE-19076: Fix NPE and TApplicationException in function related HiveMetastore methods (Marta
Kuczora, via Peter Vary)


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

Branch: refs/heads/master
Commit: 820db608f2878dde1d9c7b3fa3fbfdb3564710d6
Parents: 5eed779
Author: Marta Kuczora <kuczoram@cloudera.com>
Authored: Tue Apr 10 15:48:20 2018 +0200
Committer: Peter Vary <pvary@cloudera.com>
Committed: Tue Apr 10 15:48:20 2018 +0200

----------------------------------------------------------------------
 .../hadoop/hive/metastore/HiveMetaStore.java    |  49 +++-
 .../hive/metastore/HiveMetaStoreClient.java     |   3 +
 .../hive/metastore/client/TestFunctions.java    | 258 +++----------------
 3 files changed, 91 insertions(+), 219 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/820db608/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 c52d337..65ca63c 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
@@ -6767,6 +6767,15 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     }
 
     private void validateFunctionInfo(Function func) throws InvalidObjectException, MetaException
{
+      if (func == null) {
+        throw new MetaException("Function cannot be null.");
+      }
+      if (func.getFunctionName() == null) {
+        throw new MetaException("Function name cannot be null.");
+      }
+      if (func.getDbName() == null) {
+        throw new MetaException("Database name in Function cannot be null.");
+      }
       if (!MetaStoreUtils.validateName(func.getFunctionName(), null)) {
         throw new InvalidObjectException(func.getFunctionName() + " is not a valid object
name");
       }
@@ -6774,6 +6783,12 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       if (className == null) {
         throw new InvalidObjectException("Function class name cannot be null");
       }
+      if (func.getOwnerType() == null) {
+        throw new MetaException("Function owner type cannot be null.");
+      }
+      if (func.getFunctionType() == null) {
+        throw new MetaException("Function type cannot be null.");
+      }
     }
 
     @Override
@@ -6826,11 +6841,17 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     public void drop_function(String dbName, String funcName)
         throws NoSuchObjectException, MetaException,
         InvalidObjectException, InvalidInputException {
+      if (funcName == null) {
+        throw new MetaException("Function name cannot be null.");
+      }
       boolean success = false;
       Function func = null;
       RawStore ms = getMS();
       Map<String, String> transactionalListenerResponses = Collections.emptyMap();
       String[] parsedDbName = parseDbName(dbName, conf);
+      if (parsedDbName[DB_NAME] == null) {
+        throw new MetaException("Database name cannot be null.");
+      }
       try {
         ms.openTransaction();
         func = ms.getFunction(parsedDbName[CAT_NAME], parsedDbName[DB_NAME], funcName);
@@ -6876,14 +6897,18 @@ public class HiveMetaStore extends ThriftHiveMetastore {
 
     @Override
     public void alter_function(String dbName, String funcName, Function newFunc) throws TException
{
-      validateFunctionInfo(newFunc);
+      String[] parsedDbName = parseDbName(dbName, conf);
+      validateForAlterFunction(parsedDbName[DB_NAME], funcName, newFunc);
       boolean success = false;
       RawStore ms = getMS();
-      String[] parsedDbName = parseDbName(dbName, conf);
       try {
         ms.openTransaction();
         ms.alterFunction(parsedDbName[CAT_NAME], parsedDbName[DB_NAME], funcName, newFunc);
         success = ms.commitTransaction();
+      } catch (InvalidObjectException e) {
+        // Throwing MetaException instead of InvalidObjectException as the InvalidObjectException
+        // is not defined for the alter_function method in the Thrift interface.
+        throwMetaException(e);
       } finally {
         if (!success) {
           ms.rollbackTransaction();
@@ -6891,6 +6916,23 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       }
     }
 
+    private void validateForAlterFunction(String dbName, String funcName, Function newFunc)
+        throws MetaException {
+      if (dbName == null || funcName == null) {
+        throw new MetaException("Database and function name cannot be null.");
+      }
+      try {
+        validateFunctionInfo(newFunc);
+      } catch (InvalidObjectException e) {
+        // The validateFunctionInfo method is used by the create and alter function methods
as well
+        // and it can throw InvalidObjectException. But the InvalidObjectException is not
defined
+        // for the alter_function method in the Thrift interface, therefore a TApplicationException
+        // will occur at the caller side. Re-throwing the InvalidObjectException as MetaException
+        // would eliminate the TApplicationException at caller side.
+        throw newMetaException(e);
+      }
+    }
+
     @Override
     public List<String> get_functions(String dbName, String pattern)
         throws MetaException {
@@ -6938,6 +6980,9 @@ public class HiveMetaStore extends ThriftHiveMetastore {
 
     @Override
     public Function get_function(String dbName, String funcName) throws TException {
+      if (dbName == null || funcName == null) {
+        throw new MetaException("Database and function name cannot be null.");
+      }
       startFunction("get_function", ": " + dbName + "." + funcName);
 
       RawStore ms = getMS();

http://git-wip-us.apache.org/repos/asf/hive/blob/820db608/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
index 95a3767..8677718 100644
--- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
+++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
@@ -2668,6 +2668,9 @@ public class HiveMetaStoreClient implements IMetaStoreClient, AutoCloseable
{
 
   @Override
   public void createFunction(Function func) throws TException {
+    if (func == null) {
+      throw new MetaException("Function cannot be null.");
+    }
     if (!func.isSetCatName()) func.setCatName(getDefaultCatalog(conf));
     client.create_function(func);
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/820db608/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
index 9857c4e..b5705f9 100644
--- a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
+++ b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
@@ -37,9 +37,7 @@ import org.apache.hadoop.hive.metastore.client.builder.CatalogBuilder;
 import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
 import org.apache.hadoop.hive.metastore.client.builder.FunctionBuilder;
 import org.apache.hadoop.hive.metastore.minihms.AbstractMetaStoreService;
-import org.apache.thrift.TApplicationException;
 import org.apache.thrift.TException;
-import org.apache.thrift.transport.TTransportException;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -208,74 +206,39 @@ public class TestFunctions extends MetaStoreClientTest {
     client.createFunction(function);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
+  public void testCreateFunctionNullFunction() throws Exception {
+    client.createFunction(null);
+  }
+
+  @Test(expected = MetaException.class)
   public void testCreateFunctionNullFunctionName() throws Exception {
     Function function = testFunctions[0];
     function.setFunctionName(null);
-
-    try {
-      client.createFunction(function);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.createFunction(function);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testCreateFunctionNullDatabaseName() throws Exception {
     Function function = testFunctions[0];
     function.setDbName(null);
-
-    try {
-      client.createFunction(function);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.createFunction(function);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testCreateFunctionNullOwnerType() throws Exception {
     Function function = testFunctions[0];
     function.setFunctionName("test_function_2");
     function.setOwnerType(null);
-
-    try {
-      client.createFunction(function);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.createFunction(function);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testCreateFunctionNullFunctionType() throws Exception {
     Function function = testFunctions[0];
     function.setFunctionName("test_function_2");
     function.setFunctionType(null);
-
-    try {
-      client.createFunction(function);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.createFunction(function);
   }
 
   @Test(expected = NoSuchObjectException.class)
@@ -331,18 +294,9 @@ public class TestFunctions extends MetaStoreClientTest {
     client.getFunction(OTHER_DATABASE, function.getFunctionName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testGetFunctionNullDatabase() throws Exception {
-    try {
-      client.getFunction(null, OTHER_DATABASE);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws MetaException
-      Assert.fail("Expected an NullPointerException or MetaException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (MetaException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.getFunction(null, OTHER_DATABASE);
   }
 
   @Test(expected = MetaException.class)
@@ -371,32 +325,14 @@ public class TestFunctions extends MetaStoreClientTest {
     client.dropFunction(OTHER_DATABASE, function.getFunctionName());
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testDropFunctionNullDatabase() throws Exception {
-    try {
-      client.dropFunction(null, "no_such_function");
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.dropFunction(null, "no_such_function");
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testDropFunctionNullFunctionName() throws Exception {
-    try {
-      client.dropFunction(DEFAULT_DATABASE, null);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.dropFunction(DEFAULT_DATABASE, null);
   }
 
   @Test
@@ -601,190 +537,78 @@ public class TestFunctions extends MetaStoreClientTest {
     client.alterFunction(OTHER_DATABASE, originalFunction.getFunctionName(), newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullDatabase() throws Exception {
     Function newFunction = getNewFunction();
-
-    try {
-      client.alterFunction(null, OTHER_DATABASE, newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(null, OTHER_DATABASE, newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullFunctionName() throws Exception {
     Function newFunction = getNewFunction();
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, null, newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, null, newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullFunction() throws Exception {
     Function originalFunction = testFunctions[1];
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, originalFunction.getFunctionName(), null);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, originalFunction.getFunctionName(), null);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionInvalidNameInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setFunctionName("test_function_2;");
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // InvalidObjectException, remote throws TApplicationException
-      Assert.fail("Expected an InvalidObjectException or TApplicationException to be thrown");
-    } catch (InvalidObjectException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TApplicationException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionEmptyNameInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setFunctionName("");
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // InvalidObjectException, remote throws TApplicationException
-      Assert.fail("Expected an InvalidObjectException or TApplicationException to be thrown");
-    } catch (InvalidObjectException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TApplicationException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullClassInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setClassName(null);
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // InvalidObjectException, remote throws TApplicationException
-      Assert.fail("Expected an InvalidObjectException or TApplicationException to be thrown");
-    } catch (InvalidObjectException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TApplicationException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullFunctionNameInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setFunctionName(null);
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullDatabaseNameInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setDbName(null);
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullOwnerTypeInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setOwnerType(null);
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNullFunctionTypeInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setFunctionType(null);
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // NullPointerException, remote throws TTransportException
-      Assert.fail("Expected an NullPointerException or TTransportException to be thrown");
-    } catch (NullPointerException exception) {
-      // Expected exception - Embedded MetaStore
-    } catch (TTransportException exception) {
-      // Expected exception - Remote MetaStore
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testAlterFunctionNoSuchDatabaseInNew() throws Exception {
     Function newFunction = getNewFunction();
     newFunction.setDbName("no_such_database");
-
-    try {
-      client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
-      // TODO: Should have a check on the server side. Embedded metastore throws
-      // InvalidObjectException, remote throws TApplicationException
-      Assert.fail("Expected an InvalidObjectException or TApplicationException to be thrown");
-    } catch (InvalidObjectException exception) {
-      // Expected exception - Embedded MetaStore
-      exception.printStackTrace();
-    } catch (TApplicationException exception) {
-      // Expected exception - Remote MetaStore
-      exception.printStackTrace();
-    }
+    client.alterFunction(DEFAULT_DATABASE, "test_function_to_find_2", newFunction);
   }
 
   @Test(expected = MetaException.class)


Mime
View raw message