Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/23963 )
Change subject: IMPALA-14575: Add constant handling for Hive GenericUDFs ...................................................................... Patch Set 12: (6 comments) http://gerrit.cloudera.org:8080/#/c/23963/12/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/23963/12/common/thrift/Frontend.thrift@71 PS12, Line 71: Hive nit: Hive UDF executor? http://gerrit.cloudera.org:8080/#/c/23963/12/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/23963/12/fe/src/main/java/org/apache/impala/hive/executor/HiveGenericJavaFunction.java@1 PS12, Line 1: // Licensed to the Apache Software Foundation (ASF) under one nit: indent http://gerrit.cloudera.org:8080/#/c/23963/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfInputHandler.java File fe/src/main/java/org/apache/impala/hive/executor/HiveUdfInputHandler.java: http://gerrit.cloudera.org:8080/#/c/23963/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfInputHandler.java@26 PS12, Line 26: import org.apache.hadoop.hive.ql.exec.UDF; nit: lot of unused imports http://gerrit.cloudera.org:8080/#/c/23963/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfInputHandler.java@170 PS12, Line 170: public void fillConstArgArray(Object[] inputArgs) This is pretty similar to HiveUdfExecutorLegacy.java:77. Could you please extract it as a common method? http://gerrit.cloudera.org:8080/#/c/23963/12/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfInputHandler.java@226 PS12, Line 226: e.printStackTrace(System.err); We usually don't print out the stacktrace. http://gerrit.cloudera.org:8080/#/c/23963/12/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test File testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test: http://gerrit.cloudera.org:8080/#/c/23963/12/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@421 PS12, Line 421: ==== Could you please add a test where const and non-const arguments are mixed, like (const, non-const, const, non-const )or a similar random pattern? -- To view, visit http://gerrit.cloudera.org:8080/23963 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a6ca8c0bab499dffed88bb9786753da559af4c5 Gerrit-Change-Number: 23963 Gerrit-PatchSet: 12 Gerrit-Owner: Balazs Hevele <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Tue, 24 Feb 2026 12:39:32 +0000 Gerrit-HasComments: Yes
