Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13363 )

Change subject: IMPALA-7369: part 1: Implement TRUNC, DATE_TRUNC, EXTRACT, 
DATE_PART functions for DATE
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/benchmarks/date-benchmark.cc
File be/src/benchmarks/date-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/benchmarks/date-benchmark.cc@36
PS4, Line 36:
Nice speedup!


http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/runtime/date-test.cc
File be/src/runtime/date-test.cc:

http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/runtime/date-test.cc@669
PS4, Line 669: DateValue dv(0, 1, 1);
Can you add a comment like "The following loop iterates through all valid 
dates"?


http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/runtime/date-value.cc
File be/src/runtime/date-value.cc:

http://gerrit.cloudera.org:8080/#/c/13363/4/be/src/runtime/date-value.cc@120
PS4, Line 120:   // These magic numbers work if epoch year is 1970 and the 
supported date range is
             :   // 0000-01-01..9999-12-31.
             :   int tmp = days_since_epoch_ * 400 + 287811200;
             :   int first_year = (tmp - 591) / 146097;
             :   int last_year = (tmp + 288) / 146097;
Can you add some explanation about the formula?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I843358a45eb5faa2c134994600546fc1d0a797c8
Gerrit-Change-Number: 13363
Gerrit-PatchSet: 3
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: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 May 2019 13:22:36 +0000
Gerrit-HasComments: Yes

Reply via email to