impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tianyi Wang (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5867: Fix bugs parsing 2-digit year
Date Fri, 01 Sep 2017 21:48:35 GMT
Tianyi Wang has posted comments on this change.

Change subject: IMPALA-5867: Fix bugs parsing 2-digit year

Patch Set 1:


Refactored and changed a few logic to make it a little bit closer to Hive.
File be/src/runtime/

Line 340
> Can you preserve this comment? I think you moved this logic down to l.370?

Line 345:       if (dt_result.realign_year) {
> This function is getting pretty big. Can we factor out the year realignment

Line 346:         // In SimpleDateFormat, the century for 2-digit-year breaks at current_time
- 80 years.
> Long line.

PS1, Line 353: boost::gregorian::gregorian_calendar
> This might be clearer with a "using boost::gregorian::gregorian_calendar" u

Line 370:       if (d->year() < 1400 || d->year() > 9999) {
> Boost still won't throw an exception when you add -1 days to 1400-01-01. Bu
Changed to a single call to year(). It looks stupid but I don't know other ways to check if
it's in range.
File be/src/runtime/

Line 484:     (TimestampTC("yy-MM-dd", "00-02-29", false, true, false, 2000, 2, 29))
> Mention this is testing the leap-year edge-case.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4f430caea88b6c33f8050a1984ee0ee32ecb0a1
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <>
Gerrit-Reviewer: Tianyi Wang <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message