Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/12481 )
Change subject: IMPALA-7368: Add initial support for DATE type ...................................................................... Patch Set 17: (3 comments) http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG@20 PS11, Line 20: - Explicit casting between DATE and other types: : - from STRING > This seems to be stricter than CAST() in postgres, mysql, and hive (see htt Fixed it to allow and silently truncate the time component. http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG@38 PS11, Line 38: > DATE, STRING -> TIMESTAMP and DATE -> TIMESTAMP : implicit conversions are now all possible, the existing function : overload resolution logic is not adequate anymore. : For example, it resolves the > Should we be emitting a WARNING for any out-of-range values? it seems surpr I've added warnings to indicate that DATE->TIMESTAMP, STRING->DATE, TIMESTAMP->DATE conversions failed. Adding warning to STRING->TIMESTAMP conversions needs more work. Since these conversions are not related to the DATE type I'd prefer to fix them separately. http://gerrit.cloudera.org:8080/#/c/12481/11/fe/src/main/java/org/apache/impala/catalog/Function.java File fe/src/main/java/org/apache/impala/catalog/Function.java: http://gerrit.cloudera.org:8080/#/c/12481/11/fe/src/main/java/org/apache/impala/catalog/Function.java@191 PS11, Line 191: // Compares this to 'other' for 'mode'. > I'm afraid about introducing new semantics here which we'll need to break a Thanks for putting this together! - Impala already works as Hive 3.1 in rows 8-11, 13-14, 17, 20 and 22. - I've changed STRING->DATE conversions to accept (and silently truncate) the time component. This fixes rows 1-7, 12 and 21. - Row 15, 16 and 19 happens because Impala and PostgreSQL convert STRING, DATE, TIMESTAMP parameters to TIMESTAMP, while Hive converts them to STRING. Fixing these might not be possible w/o breaking backward compatibility. - Row 18 returns an error in Hive 2.1, and probably fails in Hive 3.1 as well (haven't tried it though). Impala and PostgreSQL convert DATE and TIMESTAMP parameters to TIMESTAMP. - Impala's behavior in row 23 is expected as valid timestamps in Impala start with year 1400. Adding YEAR(DATE) won't fix this problem either as we will still have to resolve YEAR('0009-02-15') to YEAR(TIMESTAMP) instead of YEAR(DATE) not to break backward compatibility. Users will have to be more implicit to avoid this issue and call YEAR(DATE '0009-02-15'). The best we can do is to issue a warning (which I haven't done here as it requires quite bit of work and technically it is a TIMESTAMP issue not a DATE issue. I'd prefer to do it in a separate patch.) -- 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: 17 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: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Thu, 04 Apr 2019 19:48:52 +0000 Gerrit-HasComments: Yes
