Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12481 )
Change subject: IMPALA-7368: Add initial support for DATE type ...................................................................... Patch Set 16: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/12481/11/be/src/exprs/conditional-functions-ir.cc File be/src/exprs/conditional-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/12481/11/be/src/exprs/conditional-functions-ir.cc@37 PS11, Line 37: IS_NULL_COMPUTE_ > Done Sorry for the churn here induced by my original comment. I do think there's something worth revisiting later just as a maintainability tihng - there would be some value in infrastructure to "stamp out" these more mechanical aspects of a data type, but it requires some thought. E.g. for something like a numeric type, where there's a set of common operations and functions, it seems like we should be able to define a new numeric type in one place and have the required definitions added automatically elsewhere, rather than having to chase down all the locations in the codebase. -- To view, visit http://gerrit.cloudera.org:8080/12481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea8155ef09557e0afa2f8b2d0b2dc9d0896dc30f Gerrit-Change-Number: 12481 Gerrit-PatchSet: 16 Gerrit-Owner: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 02 Apr 2019 17:49:56 +0000 Gerrit-HasComments: Yes