Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/19177 )
Change subject: WIP IMPALA-11718: Improve support for Hive Generic UDFs ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java File fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java: http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@91 PS6, Line 91: // GenericUDF is instantiated even if initalize() is not called later to typo: initialize http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java: http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java@132 PS6, Line 132: String uri = fn.getResourceUris().get( 0).getUri(); nit: extra spaces before 0 http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfLoader.java@133 PS6, Line 133: LOG.info("uri: " + uri + "localLibPath: " + localLibPath); nit: missing space before localLibPath http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/service/JniFrontend.java@422 PS6, Line 422: String returnTypeStr = fn.isGeneric() ? "GENERIC" : fn.getReturnType().toString(); Could be a member function called getReturnTypeAsString(), or make a new specialization from Type as ReturnType which could add a static instance of GENERIC http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/util/FunctionUtils.java File fe/src/main/java/org/apache/impala/util/FunctionUtils.java: http://gerrit.cloudera.org:8080/#/c/19177/6/fe/src/main/java/org/apache/impala/util/FunctionUtils.java@117 PS6, Line 117: LOG.info("Failed to match generic UDF: " + desc.functionName().toString() + " error: " + ex.getMessage()); nit: functionName is already a string -- To view, visit http://gerrit.cloudera.org:8080/19177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I692d402784fa91d1256f5d57fa16ac49728447c8 Gerrit-Change-Number: 19177 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 28 Nov 2022 09:31:21 +0000 Gerrit-HasComments: Yes
