Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16314 )
Change subject: IMPALA-7658: Proper codegen for HiveUdfCall ...................................................................... Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/16314/10/be/src/exprs/hive-udf-call.cc File be/src/exprs/hive-udf-call.cc: http://gerrit.cloudera.org:8080/#/c/16314/10/be/src/exprs/hive-udf-call.cc@266 PS10, Line 266: // TODO: Check if Timestamp works. It is currently not supported in Hive > Also, I don't think we really add any new uncertainty about timestamps, it I understand that you didn't add any new code to test, I'm saying you need to add a test for a more subtle reason. Your argument that the patch is correct is (I think) that Java UDFs don't support date or timestamp and therefore we don't need code that handle that case. I think that argument is valid. I'd just like you to write or show me a test that proves that we can't create or execute a Java UDF with date or timestamp. I took a look just now and I can't find one. Also if we have a test that proves it, we'll know if a later change breaks it. Ideally we would have added this test earlier when the date or timestamp patch was added, but it looks like we didn't, so we should fix the gap. -- To view, visit http://gerrit.cloudera.org:8080/16314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f994dac550f297ed3c88491816403f237d4d747 Gerrit-Change-Number: 16314 Gerrit-PatchSet: 10 Gerrit-Owner: Daniel Becker <daniel.bec...@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: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 27 Aug 2020 19:33:39 +0000 Gerrit-HasComments: Yes
