impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4549: consistently treat 9999 as upper bound for timestamp year
Date Thu, 12 Jan 2017 00:40:04 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4549: consistently treat 9999 as upper bound for timestamp year
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5665/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 1458:   TestIsNull("cast('10000-01-01 00:00:00' as timestamp)", TYPE_TIMESTAMP);
> This test case passes both with and without this patch. (I don't think we a
Agreed, I just added these while I was here since we didn't seem to have this coverage.

We test that case further down (where I made the mistake with the 99s).


Line 1478:   TestTimestampValue("cast(-17987443200 as timestamp)",
> I would add 2 more cases:
Done. One case was already present above so moved it down here.


Line 1503: 
> intentional blank line?
Done


PS2, Line 3612: 23:99:99
> Is this a mistake?
Oops, apparently I was thinking in decimal time for a second.


-- 
To view, visit http://gerrit.cloudera.org:8080/5665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf23b40833017789d879e5da7bb10384129e2d10
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message