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 <kuczo...@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)
