This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch branch-4.1.1 in repository https://gitbox.apache.org/repos/asf/impala.git
commit 7dce6bc8f2051aba533073f5beca99867e730a09 Author: Csaba Ringhofer <[email protected]> AuthorDate: Fri Jun 10 20:18:14 2022 +0200 IMPALA-11342: Fix class loading in Hive UDFs' constructors Loading new classes from the same jar in the constructor of UDFs did not work in the catalog because the URLClassLoader was closed too early. Extended the lifecycle of the class loader a bit to let the catalog finish all initialisation. Note that the instantiation of legacy Hive UDFs doesn't seem necessary in the catalog, we can get all relevant info from the class. Generic UDFs do need to be instantiated to be able to call initialize(). Testing: - added new classes to load in test UDFs and loaded these in constructor / initialize() - ran the Hive UDF ee tests Merge conflicts in branch-4.1: - HiveJavaFunctionFactoryImpl.java ignores the case for GenericUDF - Ignores changes in GenericImportsNearbyClassesUdf.java Change-Id: If16e38b8fc3b2577a5d32104ea9e6948b9562e24 Reviewed-on: http://gerrit.cloudera.org:8080/18611 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/19009 Reviewed-by: Csaba Ringhofer <[email protected]> --- .../hive/executor/HiveJavaFunctionFactoryImpl.java | 20 +++++++++++--------- .../apache/impala/hive/executor/HiveUdfLoader.java | 12 ++++-------- .../org/apache/impala/hive/executor/UdfExecutor.java | 2 +- .../org/apache/impala/ImportsNearbyClassesUdf.java | 5 +++++ ...rbyClassesUdf.java => UtilForUdfConstructor.java} | 12 ++++-------- ...arbyClassesUdf.java => UtilForUdfInitialize.java} | 12 ++++-------- 6 files changed, 29 insertions(+), 34 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java b/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java index c90b09bbe..44131874f 100644 --- a/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java +++ b/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java @@ -44,15 +44,17 @@ public class HiveJavaFunctionFactoryImpl implements HiveJavaFunctionFactory { checkValidFunction(hiveFn); String jarUri = hiveFn.getResourceUris().get(0).getUri(); String fnName = hiveFn.getDbName() + "." + hiveFn.getFunctionName(); - HiveUdfLoader javaClass = HiveUdfLoader.createWithLocalPath(localLibPath, hiveFn); - switch (javaClass.getUDFClassType()) { - case UDF: - return new HiveLegacyJavaFunction(javaClass.getUDFClass(), hiveFn, retType, - paramTypes); - default: - throw new CatalogException("Function " + fnName + ": The class " - + jarUri + " does not derive " - + "from a known supported Hive UDF class (UDF)."); + try (HiveUdfLoader javaClass + = HiveUdfLoader.createWithLocalPath(localLibPath, hiveFn)) { + switch (javaClass.getUDFClassType()) { + case UDF: + return new HiveLegacyJavaFunction(javaClass.getUDFClass(), hiveFn, retType, + paramTypes); + default: + throw new CatalogException("Function " + fnName + ": The class " + + jarUri + " does not derive " + + "from a known supported Hive UDF class (UDF)."); + } } } diff --git a/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java b/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java index 15f84f54d..9eaa47c61 100644 --- a/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java +++ b/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java @@ -38,7 +38,7 @@ import org.apache.log4j.Logger; * Class responsible for the Java reflection needed to fetch the UDF * class and function. */ -public class HiveUdfLoader { +public class HiveUdfLoader implements AutoCloseable { private static final Logger LOG = Logger.getLogger(HiveUdfLoader.class); private final Class<?> udfClass_; @@ -58,24 +58,20 @@ public class HiveUdfLoader { * more classes, so we allow this flexibility. In this case, the caller is * responsible to call "close" on this object. */ - public HiveUdfLoader(String localJarPath, String className, - boolean persistLoader) throws CatalogException { + public HiveUdfLoader(String localJarPath, String className) throws CatalogException { LOG.debug("Loading UDF '" + className + "' from " + localJarPath); // If the localJarPath is not set, we use the System ClassLoader which // does not need to be tracked for closing. classLoader_ = getClassLoader(localJarPath); udfClass_ = loadUDFClass(className, classLoader_); classType_ = FunctionUtils.getUDFClassType(udfClass_); - - if (!persistLoader) { - close(); - } } public Class<?> getUDFClass() { return udfClass_; } + @Override public void close() { // We only need to close URLClassLoaders. If no jar was present at instantiation, // it uses the SystemClassLoader (leaving this in for legacy purposes, but I'm not @@ -152,7 +148,7 @@ public class HiveUdfLoader { } localJarPathString = localJarPath.toString(); } - return new HiveUdfLoader(localJarPathString, fn.getClassName(), false); + return new HiveUdfLoader(localJarPathString, fn.getClassName()); } catch (Exception e) { String errorMsg = "Could not load class " + fn.getClassName() + " from " + "jar " + uri + ": " + e.getMessage(); diff --git a/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java b/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java index 79f225e70..3a15ec35a 100644 --- a/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java +++ b/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java @@ -64,7 +64,7 @@ public class UdfExecutor { } try { checkValidRequest(request); - udfLoader_ = new HiveUdfLoader(location, request.fn.scalar_fn.symbol, true); + udfLoader_ = new HiveUdfLoader(location, request.fn.scalar_fn.symbol); hiveUdfExecutor_ = createHiveUdfExecutor(request, udfLoader_); LOG.debug("Loaded UDF '" + request.fn.scalar_fn.symbol + "' from " + request.local_location); diff --git a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java b/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java index 05fbdd7b6..3aa31e9b1 100644 --- a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java +++ b/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java @@ -27,6 +27,11 @@ import org.apache.hadoop.io.Text; * the class loader. */ public class ImportsNearbyClassesUdf extends UDF { + public ImportsNearbyClassesUdf() { + // Ensure that new classes can be loaded in constructor. + UtilForUdfConstructor.getHello(); + } + public Text evaluate(Text para) throws ParseException { return new Text(UtilForUdf.getHello()); } diff --git a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java b/java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfConstructor.java similarity index 75% copy from java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java copy to java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfConstructor.java index 05fbdd7b6..b0534bbdb 100644 --- a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java +++ b/java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfConstructor.java @@ -21,13 +21,9 @@ import java.text.ParseException; import org.apache.hadoop.hive.ql.exec.UDF; import org.apache.hadoop.io.Text; -/** - * Helper for a regression test for IMPALA-8016: this - * uses another class (from the same jar) at evaluate() time, needing - * the class loader. - */ -public class ImportsNearbyClassesUdf extends UDF { - public Text evaluate(Text para) throws ParseException { - return new Text(UtilForUdf.getHello()); +/** IMPALA-11342 regression test: Helper for ImportNearbyClasses. */ +public class UtilForUdfConstructor extends UDF { + public static String getHello() { + return "Hello"; } } diff --git a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java b/java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfInitialize.java similarity index 75% copy from java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java copy to java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfInitialize.java index 05fbdd7b6..3118a3eaa 100644 --- a/java/test-hive-udfs/src/main/java/org/apache/impala/ImportsNearbyClassesUdf.java +++ b/java/test-hive-udfs/src/main/java/org/apache/impala/UtilForUdfInitialize.java @@ -21,13 +21,9 @@ import java.text.ParseException; import org.apache.hadoop.hive.ql.exec.UDF; import org.apache.hadoop.io.Text; -/** - * Helper for a regression test for IMPALA-8016: this - * uses another class (from the same jar) at evaluate() time, needing - * the class loader. - */ -public class ImportsNearbyClassesUdf extends UDF { - public Text evaluate(Text para) throws ParseException { - return new Text(UtilForUdf.getHello()); +/** IMPALA-11342 regression test: Helper for ImportNearbyClasses. */ +public class UtilForUdfInitialize extends UDF { + public static String getHello() { + return "Hello"; } }
