diff --git standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java index 8a5de09..9615371 100644 --- standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java +++ standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java @@ -6328,6 +6328,15 @@ private static MetaException newMetaException(Exception e) { } 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"); } @@ -6335,6 +6344,12 @@ private void validateFunctionInfo(Function func) throws InvalidObjectException, 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 @@ -6386,6 +6401,9 @@ public void create_function(Function func) throws TException { public void drop_function(String dbName, String funcName) throws NoSuchObjectException, MetaException, InvalidObjectException, InvalidInputException { + if (dbName == null || funcName == null) { + throw new MetaException("Database and function name cannot be null."); + } boolean success = false; Function func = null; RawStore ms = getMS(); @@ -6435,13 +6453,17 @@ public void drop_function(String dbName, String funcName) @Override public void alter_function(String dbName, String funcName, Function newFunc) throws TException { - validateFunctionInfo(newFunc); + validateForAlterFunction(dbName, funcName, newFunc); boolean success = false; RawStore ms = getMS(); try { ms.openTransaction(); ms.alterFunction(dbName, 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(); @@ -6449,6 +6471,23 @@ public void alter_function(String dbName, String funcName, Function newFunc) thr } } + 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 get_functions(String dbName, String pattern) throws MetaException { @@ -6492,6 +6531,9 @@ public GetAllFunctionsResponse get_all_functions() @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(); diff --git standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java index d504f34..70a767f 100644 --- standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java +++ standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java @@ -33,8 +33,6 @@ 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.transport.TTransportException; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -202,74 +200,39 @@ public void testCreateFunctionEmptyName() throws Exception { 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) @@ -325,18 +288,9 @@ public void testGetFunctionNoSuchFunctionInThisDatabase() throws Exception { 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) @@ -365,32 +319,14 @@ public void testDropFunctionNoSuchFunctionInThisDatabase() throws Exception { 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 @@ -595,190 +531,78 @@ public void testAlterFunctionNoSuchFunctionInThisDatabase() throws Exception { 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)