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 9: (2 comments) Do you mean that, with these code changes, you can scan HBase date tables? Or that we should have negative tests for HBase? I think we should have test coverage but I'm open to splitting that out into a separate patch if that helps this get in faster. http://gerrit.cloudera.org:8080/#/c/12481/6/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java File fe/src/main/java/org/apache/impala/analysis/DateLiteral.java: http://gerrit.cloudera.org:8080/#/c/12481/6/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java@127 PS6, Line 127: return daysSinceEpoch_ - other.daysSinceEpoch_; > Done. At the very beginning 'daysSinceEpoch_' was not a member and later I Makes sense! http://gerrit.cloudera.org:8080/#/c/12481/6/testdata/workloads/functional-query/queries/QueryTest/date.test File testdata/workloads/functional-query/queries/QueryTest/date.test: http://gerrit.cloudera.org:8080/#/c/12481/6/testdata/workloads/functional-query/queries/QueryTest/date.test@36 PS6, Line 36: where '2017-11-28' in (date_col) > To be honest, I didn't give this too much thought. I just copied the timest Ah ok, that makes sense. I don't think there are likely to be bugs here but it's good to be sure. -- 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: 9 Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 07 Mar 2019 00:55:30 +0000 Gerrit-HasComments: Yes
