Csaba Ringhofer 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? 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 making many branches predictable. (note that real world data is likely to be from a small range, so the current data may be more realistic than a shuffled one). Can you run the test with shuffled dates? It is not necessary to commit it, just check that there is no regression. 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) -- 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 <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 19 Jun 2019 16:51:33 +0000 Gerrit-HasComments: Yes
