Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16469 )
Change subject: IMPALA-10171: Create query options for local time related flags ...................................................................... Patch Set 4: (3 comments) LGTM aside from some comments http://gerrit.cloudera.org:8080/#/c/16469/4/be/src/exprs/timestamp-functions.h File be/src/exprs/timestamp-functions.h: http://gerrit.cloudera.org:8080/#/c/16469/4/be/src/exprs/timestamp-functions.h@112 PS4, Line 112: /// FLAGS_use_local_tz_for_unix_timestamp_conversions. If the flag is true, the These references to the flag need updating. http://gerrit.cloudera.org:8080/#/c/16469/4/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/16469/4/common/thrift/ImpalaInternalService.thrift@452 PS4, Line 452: nit: extra space. http://gerrit.cloudera.org:8080/#/c/16469/4/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test File testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test: http://gerrit.cloudera.org:8080/#/c/16469/4/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test@20 PS4, Line 20: utc local-timestamp-functions.test, no? Maybe also copy this comment to each query so it's clear that it applies to all of them. -- To view, visit http://gerrit.cloudera.org:8080/16469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c4252d1c8f8e224c1d0e8234f09374bcc0c6f68 Gerrit-Change-Number: 16469 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 17 Sep 2020 20:49:47 +0000 Gerrit-HasComments: Yes
