Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18020 )
Change subject: IMPALA-10997: Refactor Java Hive UDF code. ...................................................................... Patch Set 15: (6 comments) http://gerrit.cloudera.org:8080/#/c/18020/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaClass.java File fe/src/main/java/org/apache/impala/hive/executor/HiveJavaClass.java: http://gerrit.cloudera.org:8080/#/c/18020/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaClass.java@100 PS10, Line 100: > This parameter is not used in the function. Done http://gerrit.cloudera.org:8080/#/c/18020/10/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaClass.java@116 PS10, Line 116: > could be static Done http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java File fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java: http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/main/java/org/apache/impala/hive/executor/HiveJavaFunctionFactoryImpl.java@88 PS13, Line 88: functions are supported."); > nit: could be merged to 1 string Done http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorLegacy.java File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorLegacy.java: http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorLegacy.java@122 PS13, Line 122: return method_.invoke(udf_, inputArgs_); > I think that we could call evaluate much more efficiently with MethodHandle ack http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/test/java/org/apache/impala/hive/executor/HiveLegacyJavaFunctionTest.java File fe/src/test/java/org/apache/impala/hive/executor/HiveLegacyJavaFunctionTest.java: http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/test/java/org/apache/impala/hive/executor/HiveLegacyJavaFunctionTest.java@144 PS13, Line 144: public void TestExtractFailNoEvaluateMethods() { > nit: in the FE tests I know we always prefix the actual test functions with Done http://gerrit.cloudera.org:8080/#/c/18020/13/fe/src/test/java/org/apache/impala/hive/executor/HiveLegacyJavaFunctionTest.java@208 PS13, Line 208: function > typo Done -- To view, visit http://gerrit.cloudera.org:8080/18020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idc9572e15fbed1876412159b99dddd3fb4d37174 Gerrit-Change-Number: 18020 Gerrit-PatchSet: 15 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Fri, 28 Jan 2022 23:46:55 +0000 Gerrit-HasComments: Yes
