Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19304 )
Change subject: IMPALA-11549: Support Hive GenericUdfs that return primitive java types ...................................................................... Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19304/4//COMMIT_MSG@18 PS4, Line 18: probably inheriting in the opposite : direction would be more logical > I would prefer to keep is this way in this patch so that the changes in the Sounds good. http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java: http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@389 PS4, Line 389: if (result == null) return null; > In the original commit evaluateBinary returned null IMO, see Right, thanks. http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java: http://gerrit.cloudera.org:8080/#/c/19304/7/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java@54 PS7, Line 54: string Now it is string and binary types, right? http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java File java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java: http://gerrit.cloudera.org:8080/#/c/19304/4/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdfWithJavaReturnTypes.java@42 PS4, Line 42: * Similarly to TestGenericUdf this class also has copy in the FE. > The file cannot be simply copied because of the different package, see line Ok, thanks. http://gerrit.cloudera.org:8080/#/c/19304/4/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/19304/4/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@24 PS4, Line 24: #Test GenericUDF functions > I am not sure if this useful in this case, as the change is about the retur We could still add one or two cases so that we can see that the Java Object return type also works with multi-arg functions. http://gerrit.cloudera.org:8080/#/c/19304/7/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/19304/7/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test@107 PS7, Line 107: strings binaries -- To view, visit http://gerrit.cloudera.org:8080/19304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30679045d6693ebd35718b6f1a22aaa4963c1e63 Gerrit-Change-Number: 19304 Gerrit-PatchSet: 7 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-Comment-Date: Tue, 06 Dec 2022 15:13:57 +0000 Gerrit-HasComments: Yes
