Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20438 )

Change subject: IMPALA-12322 :fix the bug where kudu timestamp didn't work when 
scan kudu timestamp with timezone enabled;
......................................................................


Patch Set 2:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/20438/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20438/2//COMMIT_MSG@7
PS2, Line 7:
nit: remove space


http://gerrit.cloudera.org:8080/#/c/20438/2//COMMIT_MSG@7
PS2, Line 7: fix the bug where kudu timestamp didn't work when scan kudu 
timestamp with timezone enabled;
make the title shorter, and put fixing detail in separate paragraph.


http://gerrit.cloudera.org:8080/#/c/20438/2//COMMIT_MSG@11
PS2, Line 11: kudu_timezone.test
kudu_timezone.test should be ran from tests/query_test/test_kudu.py


http://gerrit.cloudera.org:8080/#/c/20438/2//COMMIT_MSG@12
PS2, Line 12: Change-Id: If2910f9609038bce51a6a1656981a460a4b22c82
nit: add blank line in front of "Change-Id"


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test:

http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@1
PS2, Line 1: #==========here some tests should run with 
-use_local_tz_for_unix_timestamp_conversions=true
Don't see this new test file been ran by test script. Should we run it from 
tests/query_test/test_kudu.py ?


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@11
PS2, Line 11: ---Query
add ==== between two queries


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@14
PS2, Line 14:
Here and below, remove blank line


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@17
PS2, Line 17:
Here and below, add ==== between two queries


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@45
PS2, Line 45: use_local_tz_for_unix_timestamp_conversions=false
should we set "use_local_tz_for_unix_timestamp_conversions"?


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@92
PS2, Line 92: +----+---------------------+
missing ---RESULTS


http://gerrit.cloudera.org:8080/#/c/20438/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timezone.test@98
PS2, Line 98:
add ==== in the end



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If2910f9609038bce51a6a1656981a460a4b22c82
Gerrit-Change-Number: 20438
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Wed, 30 Aug 2023 23:33:09 +0000
Gerrit-HasComments: Yes

Reply via email to