Baike Xia has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18958 )

Change subject: IMPALA-11563: Optimized /etc/sysconfig/clock to find the time 
zone
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18958/4/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/18958/4/be/src/exprs/timezone_db.cc@184
PS4, Line 184: \"");
> You could extract this string to a constant (or constexpr) so we wouldn't h
Done


http://gerrit.cloudera.org:8080/#/c/18958/4/be/src/exprs/timezone_db.cc@187
PS4, Line 187: erase(
> Something like 'header_len' would be clearer.
Done


http://gerrit.cloudera.org:8080/#/c/18958/4/be/src/exprs/timezone_db.cc@188
PS4, Line 188:
> Why do you prefer substr() instead of erase()? Because of the assignment we
This change was controversial and didn't make much sense, so I went back to the 
way it was before.


http://gerrit.cloudera.org:8080/#/c/18958/4/be/src/exprs/timezone_db.cc@188
PS4, Line 188:
> We could extract this to a variable, for example 'result_len'.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f80fd1817d072f8dadf288025cb9534191ca458
Gerrit-Change-Number: 18958
Gerrit-PatchSet: 7
Gerrit-Owner: Baike Xia <xiaba...@163.com>
Gerrit-Reviewer: Baike Xia <xiaba...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zjsar...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx91...@126.com>
Gerrit-Comment-Date: Mon, 30 Jan 2023 09:55:52 +0000
Gerrit-HasComments: Yes

Reply via email to