You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zihao Ye (Code Review)" <ge...@cloudera.org> on 2023/11/08 10:06:58 UTC
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Zihao Ye has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20681
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
14 files changed, 72 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/1
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 11:46:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#12).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
26 files changed, 587 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/12
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 12
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#2).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
14 files changed, 77 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 6:
(3 comments)
Thanks for working on the DST issues! I generally think that doing this for compile time predicate push down is critical while runtime filters are secondary, as they only matter if the timestamp column is a join key, which seems atypical for me.
For testing: I created a test table for Parquet, int64_timestamps_at_dst_changes. There are only a few values/tests, but it could be a good starting point.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc
File be/src/exec/kudu/kudu-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc@288
PS5, Line 288: TimestampValue ts_min;
: TimestampValue ts_max;
: if (state_->query_options().convert_kudu_utc_timestamps &&
: col_type.type == TYPE_TIMESTAMP) {
: ts_min = *reinterpret_cast<const TimestampValue*>(min);
: ts_max = *reinterpret_cast<const TimestampValue*>(max);
: ConvertLocalTimeMinStatToUTC(&ts_min);
: ConvertLocalTimeMaxStatToUTC(&ts_max);
: min = &ts_min;
: max = &ts_max;
: }
:
> > This can filter out values incorrectly during dst changes. The
Looks great!
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@573
PS5, Line 573: kuduPredicate = KuduPredicate.newComparisonPredicate(column, op, unixMicros);
> > This may not work correctly during DST change, see my other
I agree with the solution. EQ predicates could be turned to an in-list if it is ambiguous. ExprUtil.localTimestampToUnixTimeMicros could get an extra argument to define what kind of value is expected. There is an explanation of pre and post values at https://github.com/google/cctz/blob/2acd9f2b463eee05c354901a9fd5235fe06c017b/include/cctz/time_zone.h#L98
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS5, Line 485: // The filter is targeted for Kudu scan node with source timestamp truncation.
: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
: Expr toUnixTimeExpr =
> > Bloom filters with timezone conversion are tricky - the problem is
I think that it is ok to disable bloom filters in case isConvert_kudu_utc_timestamps is true and the timezone is not UTC. I don't know how frequently people use timestamp columns as join keys on Kudu tables - this may be niche use case not worth optimizing. If someone needs this to be fast, they can still rewrite their query to do the join on UTC timestamps, which should be the fastest and most precise way if all tables involved store the timestamp as UTC.
I am also ok with adding a query option.
An alternative way would be to not push down the bloom filter to Kudu but evaluate in the scanner post timezone conversion. I think that this would work correctly, but would be less optimal than pushing down to Kudu and would still need some work.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 6
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 15 Nov 2023 14:30:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 16:
Loading data failed should have been due to the incorrect constraint 'only' being used for the new test table 'timestamp_at_dst_changes', it should have used 'restrict_to'. This has been fixed.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 16
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 06:56:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 4:
(2 comments)
Thanks for your contribution. Overall looks good to me. Could you add some unit-test cases?
http://gerrit.cloudera.org:8080/#/c/20681/4/be/src/exec/kudu/kudu-util.h
File be/src/exec/kudu/kudu-util.h:
http://gerrit.cloudera.org:8080/#/c/20681/4/be/src/exec/kudu/kudu-util.h@76
PS4, Line 76: local_tz
should we set default value as UTCPTR?
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS4, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
isConvert_kudu_utc_timestamps() should apply on only Kudu node.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 09 Nov 2023 19:26:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 14: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10010/
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 00:20:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:57:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 21:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14719/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 21
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 12:24:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20681/20/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/20681/20/common/thrift/ImpalaService.thrift@881
PS20, Line 881:
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 20
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 11:56:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14390/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Nov 2023 09:21:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#5).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-util-ir.cc
M be/src/exec/kudu/kudu-util.cc
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
19 files changed, 92 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/5
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 17:29:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19:
I'm confused why these changes could affect test_orc_stats, but I noticed another patch that encountered the same situation: https://jenkins.impala.io/job/gerrit-verify-dryrun/10031/
Could it be due to some changes causing this test to become unstable?
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 07:15:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#14).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 591 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/14
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14470/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 7
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 17 Nov 2023 11:15:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#6).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
18 files changed, 160 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/6
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 6
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
(5 comments)
Thanks for your review! I indeed didn't consider the issue of daylight saving time, as the region I'm in doesn't observe it. I appreciate your reminder and will take it into consideration.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc
File be/src/exec/kudu/kudu-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc@288
PS5, Line 288: const Timezone* local_tz = state_->query_options().convert_kudu_utc_timestamps ?
: state_->local_time_zone() : UTCPTR;
:
: KuduValue* min_value;
: RETURN_IF_ERROR(CreateKuduValue(col_type, min, &min_value, local_tz));
: KUDU_RETURN_IF_ERROR(scanner_->AddConjunctPredicate(
: scanner_->GetKuduTable()->NewComparisonPredicate(
: col_name, KuduPredicate::ComparisonOp::GREATER_EQUAL, min_value)),
: BuildErrorString("Failed to add min predicate"));
:
: KuduValue* max_value;
: RETURN_IF_ERROR(CreateKuduValue(col_type, max, &max_value, local_tz));
> This can filter out values incorrectly during dst changes. The
> issue is that two utc micro values 1 hour apart can map to the same
> local timestamp, e.g. e.g. both 01:10:00 (UTC) and 02:10:00 (UTC)
> may be to 00:10:00 (local). A filter like 00:05:00 < ts < 00:15:00
> would be either converted to 02:05:00 < ts < 02:15:00 or 01:05:00 <
> ts < 01:15:00, and both would drop one of the values incorrectly.
>
> Generally we should use the smaller possible conversion for min
> value and the larger one for max value if they are ambiguous. A
> similar logic was implemented for Parquet stat filtering at
> https://github.com/apache/impala/blob/3be9b82207ddf32ca48ea1b2afbc88d8dffad8a4/be/src/exec/parquet/parquet-common.cc#L266
> , but it works the other way, we have the UTC min/max stats from
> the Parquet file, and try to get their min/max local values.
I tried using a similar approach to solve this problem, and after a simple test, it seems worked fine when converting UTC ambiguities.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exprs/timestamp-functions-ir.cc@139
PS5, Line 139: local_time_zone
> This should use time_zone_for_unix_time_conversions() instead of
> local_time_zone to assume utc if use_local_tz_for_unix_timestamp_conversions
> is false.
>
> If you need a function that behaves differently than other
> functions, then it could have a special name like ToUnixMicrosForKudu
I might consider removing this function later and implementing the conversion in a different way.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/service/impala-server.cc@371
PS5, Line 371: DEFINE_bool(convert_kudu_utc_timestamps, false,
> I don't think that we need a flag if we already have a query option, as the
Done
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@573
PS5, Line 573: kuduPredicate = KuduPredicate.newComparisonPredicate(column, op, unixMicros);
> This may not work correctly during DST change, see my other
> comments.
After testing, it is indeed problematic, and I'm working on finding a solution. For the "in list" predicate, perhaps we can add both ambiguous values for the conversion. As for binary predicates, we could handle them similar to the min_max runtime filter by using a broader range of conversion values. Do you have any suggestions regarding this?
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS5, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
: Expr toUnixTimeExpr =
: new FunctionCallExpr(functionName, Lists.newArrayList(srcExpr));
> Bloom filters with timezone conversion are tricky - the problem is
> that two different utc micro timestamps can be mapped to the same
> local timestamp, e.g. in case of dst change. If we map the local
> timestamp to a single utc timestamp during bloom filter creation,
> then it will only match with one of the possible values that would
> be converted to this timestamp, so we will incorrectly filter the
> other value.
The timezone conversion for Bloom filters is indeed tricky. Enabling both ambiguous values in the filter could potentially solve the problem, but it would require significant modifications. Currently, I'm considering disabling the Bloom filter for timezone conversion in Kudu and adding an additional query option to re-enable it in such cases. This would allow regions that do not observe daylight saving time to function properly. Another option is to discard the Bloom filter when ambiguous values are detected, although the extent of its impact is uncertain. Do you have any suggestions regarding this?
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 15 Nov 2023 12:32:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 18: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 18
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:56:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 15:
GVO failed when loading data
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 15
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 04:19:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 17:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10019/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 17
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 08:32:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20681/1/be/src/runtime/timestamp-value.inline.h
File be/src/runtime/timestamp-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/20681/1/be/src/runtime/timestamp-value.inline.h@118
PS1, Line 118: /// Interpret 'this' as a timestamp in 'local_tz' and convert to unix time in microseconds.
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Nov 2023 10:08:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 15
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 00:46:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 16:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14692/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 16
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 07:12:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 17: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 17
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 08:32:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 20:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14718/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 20
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 12:23:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#10).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
18 files changed, 339 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/10
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 10
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14483/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 9
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 09:08:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
There is another related patch https://gerrit.cloudera.org/#/c/20438/ which was submitted two months ago, but did not make progress. We could abandon that patch after this patch is merged, right?
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 10 Nov 2023 18:42:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14520/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 12
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 27 Nov 2023 06:55:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 12:
Test cases have been added.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 12
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 27 Nov 2023 06:59:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 11:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/14516/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 11
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 24 Nov 2023 11:59:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 12: Code-Review+1
(3 comments)
Great work! Only minor comments.
http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:
http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py@257
PS12, Line 257: [['to_utc_timestamp'], 'TIMESTAMP', ['TIMESTAMP', 'STRING', 'BOOLEAN'],
: "impala::TimestampFunctions::ToUtcUnambiguous"],
Maybe this should rather go to the invisible functions list?
I am not sure here - while I would prefer to not make this a supported functions, it can be useful for users in some cases.
http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@744
PS12, Line 744: */
Can you mention that now this can return a list for timestamps?
http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test:
http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test@18
PS12, Line 18: order by id
here and at other queries: "order by id" is not necessary - the rows in the results section + the actual results are ordered before comparison by default in EE tests
otherwise most tests would need order by, as Impala has no guarantee for the order of returned rows if there are multiple files in a table
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 12
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 07 Dec 2023 22:39:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 14: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 21:30:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 15:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10014/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 15
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 00:46:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 17: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10019/
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 17
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 13:07:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10024/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:57:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14373/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Nov 2023 12:03:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#4).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-util-ir.cc
M be/src/exec/kudu/kudu-util.cc
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
18 files changed, 85 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/4
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#8).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
21 files changed, 338 insertions(+), 48 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/8
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 8
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14478/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 8
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 20 Nov 2023 09:08:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
(2 comments)
Thank you for your suggestion! I agree with your point that using a timestamp column as a join key is indeed rare, and thus, making extensive modifications for this might not be worthwhile. Now that everything looks complete except for adding tests, I'll add some tests asap.
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@573
PS5, Line 573: kuduPredicate = KuduPredicate.newComparisonPredicate(column, op, unixMicros);
> I agree with the solution. EQ predicates could be turned to an in-list if i
Done
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS5, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
: Expr toUnixTimeExpr =
: new FunctionCallExpr(functionName, Lists.newArrayList(srcExpr));
> I think that it is ok to disable bloom filters in case isConvert_kudu_utc_t
Done
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 09:51:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 14:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10010/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 21:31:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 21: Verified+1 Code-Review+2
Merging manually as there was only whitespace change since last GVO + the tests seem unstable.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 21
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 13:19:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14664/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 09:53:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 14: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 14
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 10:33:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#20).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 592 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/20
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 20
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#13).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 591 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/13
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 13
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10024/
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 19:25:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 18:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14704/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 18
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 15:09:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 15: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10014/
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 15
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 03:26:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#16).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 591 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/16
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 16
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10033/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 07:19:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS4, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
> > isConvert_kudu_utc_timestamps() should apply on only Kudu node.
Don't need to rename the variable. Add a comment before this line "The filter is targeted for Kudu scan node with source
timestamp truncation."
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 10 Nov 2023 18:22:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 4:
Hi Wenzhe, could you please take a look at this patch when you have time? I think you should be quite familiar with this part.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 09 Nov 2023 09:36:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14371/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Nov 2023 10:34:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#11).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 597 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/11
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 11
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#3).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-util-ir.cc
M be/src/exec/kudu/kudu-util.cc
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
18 files changed, 83 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/3
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/14389/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Nov 2023 08:29:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 4:
(2 comments)
Thanks for the review, I will add some unit-test cases next week.
http://gerrit.cloudera.org:8080/#/c/20681/4/be/src/exec/kudu/kudu-util.h
File be/src/exec/kudu/kudu-util.h:
http://gerrit.cloudera.org:8080/#/c/20681/4/be/src/exec/kudu/kudu-util.h@76
PS4, Line 76: local_tz
> should we set default value as UTCPTR?
Done
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS4, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
> isConvert_kudu_utc_timestamps() should apply on only Kudu node.
According to the comments and related code of isTimestampTruncation, when isTimestampTruncation is true, isConvert_kudu_utc_timestamps() already only be applied to Kudu scan nodes now. However, perhaps we can rename isTimestampTruncation to make it clearer, for example, how about isKuduTimestampTruncation?
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 10 Nov 2023 10:56:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#18).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 592 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/18
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 18
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 4:
(1 comment)
> There is another related patch https://gerrit.cloudera.org/#/c/20438/ which was submitted two months ago, but did not make progress. We could abandon that patch after this patch is merged, right?
Yes, I saw it. Both patches address the same issue, but it seems like that patch doesn't have any further updates.
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/4/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS4, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
> Don't need to rename the variable. Add a comment before this line "The filt
Done
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 15 Nov 2023 12:13:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14450/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 6
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 15 Nov 2023 12:32:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#21).
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 592 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/21
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 21
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#7).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
21 files changed, 301 insertions(+), 48 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/7
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 7
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14409/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 10 Nov 2023 11:11:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 5:
(5 comments)
Left a few comments mainly about DST change troubles, as the patch doesn't seem to be prepared for those. Generally reading the values from Kudu and converting them to local time should work, but handling predicates/runtime filters correctly can be very tricky.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc
File be/src/exec/kudu/kudu-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exec/kudu/kudu-scanner.cc@288
PS5, Line 288: const Timezone* local_tz = state_->query_options().convert_kudu_utc_timestamps ?
: state_->local_time_zone() : UTCPTR;
:
: KuduValue* min_value;
: RETURN_IF_ERROR(CreateKuduValue(col_type, min, &min_value, local_tz));
: KUDU_RETURN_IF_ERROR(scanner_->AddConjunctPredicate(
: scanner_->GetKuduTable()->NewComparisonPredicate(
: col_name, KuduPredicate::ComparisonOp::GREATER_EQUAL, min_value)),
: BuildErrorString("Failed to add min predicate"));
:
: KuduValue* max_value;
: RETURN_IF_ERROR(CreateKuduValue(col_type, max, &max_value, local_tz));
This can filter out values incorrectly during dst changes. The issue is that two utc micro values 1 hour apart can map to the same local timestamp, e.g. e.g. both 01:10:00 (UTC) and 02:10:00 (UTC) may be to 00:10:00 (local). A filter like 00:05:00 < ts < 00:15:00 would be either converted to 02:05:00 < ts < 02:15:00 or 01:05:00 < ts < 01:15:00, and both would drop one of the values incorrectly.
Generally we should use the smaller possible conversion for min value and the larger one for max value if they are ambiguous. A similar logic was implemented for Parquet stat filtering at https://github.com/apache/impala/blob/3be9b82207ddf32ca48ea1b2afbc88d8dffad8a4/be/src/exec/parquet/parquet-common.cc#L266 , but it works the other way, we have the UTC min/max stats from the Parquet file, and try to get their min/max local values.
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/exprs/timestamp-functions-ir.cc@139
PS5, Line 139: local_time_zone
This should use time_zone_for_unix_time_conversions() instead of local_time_zone to assume utc if use_local_tz_for_unix_timestamp_conversions is false.
If you need a function that behaves differently than other functions, then it could have a special name like ToUnixMicrosForKudu
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20681/5/be/src/service/impala-server.cc@371
PS5, Line 371: DEFINE_bool(convert_kudu_utc_timestamps, false,
I don't think that we need a flag if we already have a query option, as the default of the query option can be set with flag default_query_options. convert_legacy_hive_parquet_utc_timestamps flag was kept for backwards compatibility, as it existed before the query option.
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@573
PS5, Line 573: kuduPredicate = KuduPredicate.newComparisonPredicate(column, op, unixMicros);
This may not work correctly during DST change, see my other comments.
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/20681/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@485
PS5, Line 485: String functionName = analyzer.getQueryOptions().isConvert_kudu_utc_timestamps() ?
: "to_unix_micros" : "utc_to_unix_micros";
: Expr toUnixTimeExpr =
: new FunctionCallExpr(functionName, Lists.newArrayList(srcExpr));
Bloom filters with timezone conversion are tricky - the problem is that two different utc micro timestamps can be mapped to the same local timestamp, e.g. in case of dst change. If we map the local timestamp to a single utc timestamp during bloom filter creation, then it will only match with one of the possible values that would be converted to this timestamp, so we will incorrectly filter the other value.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:33:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 12:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:
http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py@257
PS12, Line 257: [['to_utc_timestamp'], 'TIMESTAMP', ['TIMESTAMP', 'STRING', 'BOOLEAN'],
: "impala::TimestampFunctions::ToUtcUnambiguous"],
> Maybe this should rather go to the invisible functions list?
> I am not sure here - while I would prefer to not make this a
> supported functions, it can be useful for users in some cases.
I've moved it to the invisible functions list. I feel it might not be intuitive as a function provided to users. If needed, we can introduce functions with clearer naming and parameters in a separate Jira ticket.
http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@744
PS12, Line 744: */
> Can you mention that now this can return a list for timestamps?
Done
http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test:
http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test@18
PS12, Line 18: order by id
> here and at other queries: "order by id" is not necessary - the
> rows in the results section + the actual results are ordered before
> comparison by default in EE tests
>
> otherwise most tests would need order by, as Impala has no
> guarantee for the order of returned rows if there are multiple
> files in a table
Thank you for the reminder, all those "order by id" have been removed.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 12
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 09:59:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 13:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/14663/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 13
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 12 Dec 2023 09:08:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 16: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 16
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 08:32:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19:
There was one test failure in TestOrcStats.test_orc_stats
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 19:57:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 19:
> Patch Set 19:
>
> I'm confused why these changes could affect test_orc_stats, but I noticed another patch that encountered the same situation: https://jenkins.impala.io/job/gerrit-verify-dryrun/10031/
> Could it be due to some changes causing this test to become unstable?
I think it's unrelated to this patch. Just filed IMPALA-12630.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 19
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 07:19:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Testing:
- Add TestKuduTimestampConvert in query_test/test_kudu.py
Perform end-to-end testing in a custom cluster, including basic Kudu UTC
timestamp conversion testing, as well as checking if related predicate
pushdown and runtime filters are working correctly (even with timestamps
involving daylight saving time conversions).
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Reviewed-on: http://gerrit.cloudera.org:8080/20681
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Csaba Ringhofer <cs...@cloudera.com>
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/rat_exclude_files.txt
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
A testdata/data/timestamp_at_dst_changes.txt
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test
M tests/query_test/test_kudu.py
25 files changed, 592 insertions(+), 37 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 22
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 18:
Due to the lack of an 'only' constraint, the load of 'functional_kudu.timestamp_at_dst_changes' was skipped. This has been fixed.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 18
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:44:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Wenzhe Zhou, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20681
to look at the new patch set (#9).
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
[WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
This patch adds a query option 'convert_kudu_utc_timestamps' similar to
'convert_legacy_hive_parquet_utc_timestamps'. When enabled, it converts
UTC timestamps read from Kudu to local timestamps.
The corresponding modification also include predicate pushdown and
runtime filter. Due to the ambiguity of timestamps caused by daylight
saving time changes, it is difficult to resolve in the bloom filter.
This patch additionally introduces a query option
'disable_kudu_local_timestamp_bloom_filter' to default disable the Kudu
timestamp bloom filter after enabling time zone conversion in order to
avoid erroneously filtering out data. However, for regions that do not
observe daylight saving time, it can be set to false to re-enable the
Kudu local timestamp bloom filter.
Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
---
M be/src/exec/kudu/kudu-scanner.cc
M be/src/exec/kudu/kudu-scanner.h
M be/src/exec/kudu/kudu-util.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/function-registry/impala_functions.py
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/util/ExprUtil.java
19 files changed, 343 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/20681/9
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 9
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
[Impala-ASF-CR] [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: [WIP] IMPALA-12322: Support converting UTC timestamps read from Kudu to local time
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14484/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3
Gerrit-Change-Number: 20681
Gerrit-PatchSet: 10
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 10:25:11 +0000
Gerrit-HasComments: No