Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21735 )
Change subject: IMPALA-7086 Cache timezone in *_utc_timestamp(): ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/21735/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21735/4//COMMIT_MSG@7 PS4, Line 7: IMPALA-7086: Cache timezone name look ups the : should be after the jira ID http://gerrit.cloudera.org:8080/#/c/21735/4//COMMIT_MSG@13 PS4, Line 13: Can you add the original and the improved times? http://gerrit.cloudera.org:8080/#/c/21735/1/be/src/exprs/timestamp-functions.h File be/src/exprs/timestamp-functions.h: http://gerrit.cloudera.org:8080/#/c/21735/1/be/src/exprs/timestamp-functions.h@156 PS1, Line 156: it > Imho name can be viewed as if what is stored is a string. What about "Timez The optimization is triggered by the argument being constant, which is the timezone name. The Timezone object itself was constant even before the optimization, as the lookup always returned the same object. http://gerrit.cloudera.org:8080/#/c/21735/4/be/src/exprs/timestamp-functions.cc File be/src/exprs/timestamp-functions.cc: http://gerrit.cloudera.org:8080/#/c/21735/4/be/src/exprs/timestamp-functions.cc@103 PS4, Line 103: imestampValue ts_value = TimestampValue::FromTimestamp timezone == nullptr || context->IsArgConstant(1) is simpler http://gerrit.cloudera.org:8080/#/c/21735/4/be/src/exprs/timestamp-functions.cc@125 PS4, Line 125: ts_value_ret.UtcToLocal(*timezone); nit: I think that it is clearer to write out const Timezone* in this simple case. http://gerrit.cloudera.org:8080/#/c/21735/4/be/src/exprs/timestamp-functions.cc@150 PS4, Line 150: } el same as line 125 http://gerrit.cloudera.org:8080/#/c/21735/4/be/src/exprs/timestamp-functions.cc@168 PS4, Line 168: ts_value.ToString(), tz_string_value.DebugString()); nit: trailing space -- To view, visit http://gerrit.cloudera.org:8080/21735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icdf5ff82c5d0554333aef1bc3bba034a4cf48230 Gerrit-Change-Number: 21735 Gerrit-PatchSet: 1 Gerrit-Owner: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Comment-Date: Wed, 04 Sep 2024 09:20:10 +0000 Gerrit-HasComments: Yes
