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/04/10 13:49:18 UTC
hive git commit: HIVE-19076: Fix NPE and TApplicationException in
function related HiveMetastore methods (Marta Kuczora, via Peter Vary)
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 <ku...@cloudera.com>
Authored: Tue Apr 10 15:48:20 2018 +0200
Committer: Peter Vary <pv...@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)