Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 )
Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu ...................................................................... Patch Set 8: Code-Review+1 (11 comments) http://gerrit.cloudera.org:8080/#/c/21492/6/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/6/common/thrift/ImpalaService.thrift@883 PS6, Line 883: conversion > Done Done http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/main/java/org/apache/impala/util/ExprUtil.java File fe/src/main/java/org/apache/impala/util/ExprUtil.java: http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/main/java/org/apache/impala/util/ExprUtil.java@104 PS6, Line 104: Preconditions.checkArgument(timestampExpr.isConstant()); > Those are added here: https://github.com/apache/impala/blob/5d1bd80623324f8 Ack http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@753 PS6, Line 753: kudu-dml-with-utc-conversion > The only case where CONVERT_KUDU_UTC_TIMESTAMPS affects the plan are bloom Ack http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv File testdata/datasets/functional/schema_constraints.csv: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv@425 PS6, Line 425: # The table is used to test DST changes in timestamps, the timestamps in the table near : # DST changes in the 'America/Los_Angeles' time zone. > Done Done http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test@75 PS6, Line 75: > Is this whitespace a defect from getExplainString() ? Have you look into this extra whitespace? http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@66 PS6, Line 66: select * from utc_kudu where id > 10; > Changed the results in other tests to be always ordered by id. I can also a Just ordering in test file is fine. Than you. http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@79 PS6, Line 79: select * from utc_kudu where id > 10; > Add "order by id" for readability? Done http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@129 PS6, Line 129: select * from utc_kudu where ts_pk_col = ts_c > Done Done http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@138 PS6, Line 138: 2011-11-06 01:20:06,2011-11-06 01:20: > Add "order by id" for readability? Done http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@149 PS6, Line 149: TIMESTAMP,TIMESTAMP,INT > Add "order by id" for readability? Done http://gerrit.cloudera.org:8080/#/c/21492/6/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/21492/6/tests/query_test/test_kudu.py@108 PS6, Line 108: cls.ImpalaTestMatrix.add_mandatory_exec_option('write_kudu_utc_timestamps', 'true') > Done Done -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Zihao Ye <[email protected]> Gerrit-Comment-Date: Mon, 17 Jun 2024 16:56:15 +0000 Gerrit-HasComments: Yes
