Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16720 )
Change subject: [WIP] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate ...................................................................... Patch Set 24: (3 comments) http://gerrit.cloudera.org:8080/#/c/16720/24/be/src/runtime/date-value.cc File be/src/runtime/date-value.cc: http://gerrit.cloudera.org:8080/#/c/16720/24/be/src/runtime/date-value.cc@369 PS24, Line 369: DateValue DateValue::SubtractDays(int64_t days) const { I am not sure if this is really useful, as we already have AddDays that can be called with negative values. http://gerrit.cloudera.org:8080/#/c/16720/24/be/src/runtime/timestamp-value.cc File be/src/runtime/timestamp-value.cc: http://gerrit.cloudera.org:8080/#/c/16720/24/be/src/runtime/timestamp-value.cc@217 PS24, Line 217: add There is already an implementation to add intervals to timestamps at https://github.com/apache/impala/blob/master/be/src/exprs/timestamp-functions-ir.cc#L685 It would be good to use the same implementation, because for bit time_duration the add/sub can tricky. If I saw correctly then you only add nanoseconds, so it would be enough to add an "addNanoSecond" functions that could handle negative values too. timestamp-functions-ir.h could expose a function that does the add and we could call it from here. http://gerrit.cloudera.org:8080/#/c/16720/24/be/src/runtime/timestamp-value.cc@226 PS24, Line 226: TimestampValue(date_ + boost::gregorian::date_duration(1), this doesn't work correctly if 't' is more then one day -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 24 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 03 Dec 2020 22:29:10 +0000 Gerrit-HasComments: Yes
