Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13648 )

Change subject: IMPALA-7369: part 2: Add INTERVAL expr support and built-in 
functions for DATE
......................................................................


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13648/3/be/src/benchmarks/date-benchmark.cc@37
PS3, Line 37: 4.92X
> Nice results! Can you mention this optimization in the commit message?
Done


http://gerrit.cloudera.org:8080/#/c/13648/3/be/src/benchmarks/date-benchmark.cc@169
PS3, Line 169:   data.AddRange(DateValue(1965, 1, 1), DateValue(2020, 12, 31));
> It is possible that the sorted test data helps the current solution by maki
Done. Somehow the results are even better with shuffled test data.


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

http://gerrit.cloudera.org:8080/#/c/13648/3/be/src/runtime/date-value.cc@199
PS3, Line 199:   int m = int(days_since_jan1 / 30.5);
> Idea for +1 DCHECK: DCHECK(month_ranges[m] > days_since_jan1)
Actually it should be the other way around:
DCHECK(month_ranges[m] <= days_since_jan1)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If404bffdaf055c769e79ffa8f193bac415cfdd1a
Gerrit-Change-Number: 13648
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-Comment-Date: Thu, 20 Jun 2019 16:18:36 +0000
Gerrit-HasComments: Yes

Reply via email to