[Impala-ASF-CR] IMPALA-7086 Cache timezone in * utc timestamp():

["Csaba Ringhofer (Code Review)" <ge...@cloudera.org>]

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 <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Comment-Date: Wed, 04 Sep 2024 09:20:10 +0000
Gerrit-HasComments: Yes