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