[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 03 Aug 2018 17:45:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Reviewed-on: http://gerrit.cloudera.org:8080/11064 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 159 insertions(+), 7 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 03 Aug 2018 14:31:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2914/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 03 Aug 2018 14:31:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 03 Aug 2018 12:53:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py@54
PS7, Line 54: values[0].strip("[]")
> Options without brackets are normal if the option does not have the default
Thanks for the explanation.
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 03 Aug 2018 12:53:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11064/7/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/11064/7/common/thrift/ImpalaService.thrift@332
PS7, Line 332: overridde
> nit: overridden
Done
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py@54
PS7, Line 54: values[0].strip("[]")
> I think, values[0][1:-1] might be more correct here. In extreme cases, valu
Options without brackets are normal if the option does not have the default
value. I have changed the regexp to reflect this + added a unit test for
test_find_query_option.
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 03 Aug 2018 11:55:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#8). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 159 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/8 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 7: Code-Review+1 (1 comment) lgtm once Attila is happy http://gerrit.cloudera.org:8080/#/c/11064/7/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/11064/7/common/thrift/ImpalaService.thrift@332 PS7, Line 332: overrided nit: overridden -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 03 Aug 2018 01:19:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/7/tests/shell/test_shell_commandline.py@54
PS7, Line 54: values[0].strip("[]")
I think, values[0][1:-1] might be more correct here. In extreme cases, value
may conatain a '[' character too
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 02 Aug 2018 17:45:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/159/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 02 Aug 2018 12:39:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#7). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 140 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/7 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): > Are we sure that we plan to follow this rule in Impala? I keep it as it is We were having a discussion about this on the mailing list. I'm actually in favour of enabling this one although a lot of existing code doesn't follow it just for the sake of consistency -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 20:46:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/138/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:58:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/137/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:45:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py@53 PS5, Line 53: " > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): > flake8: E302 expected 2 blank lines, found 1 Are we sure that we plan to follow this rule in Impala? I keep it as it is for now as there was only 1 line before the change. -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:29:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11064/6/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/6/tests/shell/test_shell_commandline.py@42 PS6, Line 42: def find_query_option(key, string, strip_brackets=True): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:28:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#6). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 139 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/6 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc@1287
PS4, Line 1287: TimezoneDatabase::LocalZoneName();
> Shouldn't we check if (TimezoneDatabase::FindTimezone(timezone) == nullptr)
I think that we shouldn't worry too much about that case here. I prefer to do
the fallback when starting queries because it gives a nice error message to the
user.
Note that I plan to turn this case to a fatal error during startup in the
future when I will implement IMPALA-7365: Cache local timezone at startup.
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift@430
PS4, Line 430: TimezoneDb
> TimezoneDatabase::LocalZoneName()
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test
File testdata/workloads/functional-query/queries/QueryTest/set.test:
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@237
PS4, Line 237: 1970-01-01 01:00:00
> I'm a little confused. We should get these resutls only if implad is run wi
Ouch, I have added these a bit mindlessly. The tests worked on my local
machine, because the cluster was started with
-use_local_tz_for_unix_timestamp_conversions=true. Thanks for spotting it!
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@245
PS4, Line 245: 1969-12-31 16:00:00
> Sme as above
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@42
PS4, Line 42: =
> nit: remove spaces around '='
I think that this function can be useful in other tests too to check the value
of query options, and some may want to keep the brackets to check if the value
is the default or not.
The reason why I prefer this function to assert "X" in result.stdout is that
the stdout is often trimmed in the test's output so the actual value is often
missing from the logs.
+ I have removed the spaces.
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@50
PS4, Line 50: pattern = key + ": .*$"
: values = [s.split(":")[1][1:] for s in re.findall(pattern,
string, re.MULTILINE)]
> This can be simplified a bit:
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@714
PS4, Line 714:
:It would be nice to check that the default timezone is
the system's timezone,
:but doing this reliably on different Linux distributions
is quite hard.
> Maybe you could use 'dateutil.tz' module:
I tried to read the exact timestamps in older versions of this review, see
https://gerrit.cloudera.org/#/c/11064/1..4/tests/shell/test_shell_commandline.py
The solution should have worked on Ubuntu, and was ok on my local machine, but
the jenkins job failed with "UTC" vs "Etc/UTC". Probably the difference was
related to following symlinks differently, but I did want to dive in to this
for the sake of the test.
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Aug 2018 19:25:57 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py@53 PS5, Line 53: " flake8: E501 line too long (91 > 90 characters) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:06:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#5). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 138 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/5 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc@1287
PS4, Line 1287: TimezoneDatabase::LocalZoneName();
Shouldn't we check if (TimezoneDatabase::FindTimezone(timezone) == nullptr)
here, and fall back to UTC if it is?
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift@430
PS4, Line 430: TimezoneDb
TimezoneDatabase::LocalZoneName()
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test
File testdata/workloads/functional-query/queries/QueryTest/set.test:
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@237
PS4, Line 237: 1970-01-01 01:00:00
I'm a little confused. We should get these resutls only if implad is run with
-use_local_tz_for_unix_timestamp_conversions=true, right? Where is the startup
flag set?
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@245
PS4, Line 245: 1969-12-31 16:00:00
Sme as above
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@42
PS4, Line 42: strip_brackets
Why do you need this parameter? Any chance someome would not want to trim the
brackets?
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@42
PS4, Line 42: =
nit: remove spaces around '='
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@50
PS4, Line 50: pattern = key + ": .*$"
: values = [s.split(":")[1][1:] for s in re.findall(pattern,
string, re.MULTILINE)]
This can be simplified a bit:
pattern = '^\s*%s: (\[.*\])\s*$' % key
values = re.findall(pattern, string, re.MULTILINE)
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@714
PS4, Line 714:
:It would be nice to check that the default timezone is
the system's timezone,
:but doing this reliably on different Linux distributions
is quite hard.
Maybe you could use 'dateutil.tz' module:
https://dateutil.readthedocs.io/en/stable/tz.html
Version 2.5.2 is already pip-installed.
Something like this may work:
import dateutil.tz
assert dateutil.tz.gettz() == dateutil.tz.gettz(tzname)
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Aug 2018 14:17:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 2: (1 comment) Thanks for mentioning the ANSI SQL equivalent - I didn't know anything about that. It seems like implementing this with regular query option syntax wouldn't preclude supporting that too (assuming they are both implemented as updating a session-level option). http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9 PS2, Line 9: This change adds a new query option "timezone" which > It turned out that MySQL uses "set time_zone=tzname" with similar semantics It seems like Oracle also uses 'time_zone': https://docs.oracle.com/cd/E11882_01/server.112/e10729/ch4datetime.htm#CHDCDCBH -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 22:44:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/98/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 15:46:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 2:
(2 comments)
The future of this change in under discussion right now - there is an ANSI SQL
command "SET TIME ZONE", and it would be nice to support it in Impala, but it
expects different input than the query option I implemented here.
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9
PS2, Line 9: This change adds a new query option "timezone" which
> Yeah I kind of like the shorter name "timezone" upon further reflection.
It turned out that MySQL uses "set time_zone=tzname" with similar semantics, so
I think I will switch to that name.
http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py@706
PS2, Line 706: result_set = run_impala_shell_cmd('-q "set;"')
> Can you also add a test to tests/metadata/test_set.py that exercises the se
Done
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 30 Jul 2018 15:16:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 4: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/98/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 15:13:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#4). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M testdata/workloads/functional-query/queries/QueryTest/set.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 10 files changed, 153 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/4 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Code-Review+1 (2 comments) Will let Attila take a look too http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9 PS2, Line 9: This change adds a new query option "timezone" which > I am unsure about this - Linux uses /etc/timezone and $TZ for the same conc Yeah I kind of like the shorter name "timezone" upon further reflection. http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py@706 PS2, Line 706: assert actual_time_s <= time_limit_s, ( Can you also add a test to tests/metadata/test_set.py that exercises the server-side 'set' command? -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 22:11:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:10:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2870/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:54:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/85/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:50:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9 PS2, Line 9: This change adds a new query option "timezone" which > I wonder if something like local_timezone would be clearer? Or is that too I am unsure about this - Linux uses /etc/timezone and $TZ for the same concept. I am not against local_timezone - if everyone likes it then I can change the name in the next patch. http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@11 PS2, Line 11: The main goal is to simplify testing, but I think that > Can you briefly mention that this is meant to be a user-visible option rath Done http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@24 PS2, Line 24: cannot be entered unquoted in some contexts. : : Currently the timezone has effect in the following cases: : -function now() > Is there any potential interaction with table properties or similar? I don' isAdjustedToUTC is included in the metadata in the Parquet file's footer - the reason for doing this is that Hive (and probably Sparc?) does a local->utc conversion when writing the timestamp, and Impala needs to convert it back to see the same results. Meanwhile Impala writes and reads it without any conversion (which is much faster). "to convert on not to convert" could be decided by the writer's version and the flag until now. Newer versions of Hive may write the timestamps in a way that will need no conversion by Impala to improve scan speed - as the same writer will write different timestamps depending on configuration, the writer's version will not be enough the decide, so this information has to be included in the metadata. Writing it to table property would not work because different applications may write the files of the same table. This logic is added in https://gerrit.cloudera.org/#/c/11057/1/be/src/exec/parquet-column-readers.cc / line 412. http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@687 PS2, Line 687: // Leading/trailing " and ' characters are stripped because the / character > The stripping might need a brief explanation Done http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689 PS2, Line 689: string timezone = value; > Missing " " before ( Done http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689 PS2, Line 689: > nullptr Done http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift@331 PS2, Line 331: The default is the > This is a little unclear because the default value is "". I guess this mean Thanks for spotting the inconsistency - it remained there from a different state of the patch. http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py@62 PS2, Line 62: > Can we combine this with test_utc_timestamp_functions() to avoid restarting Done -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:50:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/85/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:16:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#3). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 138 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/3 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 2:
(8 comments)
Took
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9
PS2, Line 9: This change adds a new query option "timezone" which
I wonder if something like local_timezone would be clearer? Or is that too
verbose?
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@11
PS2, Line 11:
Can you briefly mention that this is meant to be a user-visible option rather
than a development/debugging option? I think that's the intent right?
http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@24
PS2, Line 24: In the near future Parquet timestamps's
: isAdjustedToUTC property will be supported, which will
: decide whether to do utc->local conversion on a per
: file+column basis.
Is there any potential interaction with table properties or similar? I don't
know if there's ever a need to specify this per-table rather than per-user.
http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@687
PS2, Line 687: string timezone =
StripSuffixString(StripPrefixString(value, "\""), "\"");
The stripping might need a brief explanation
http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689
PS2, Line 689: NULL
nullptr
http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689
PS2, Line 689: if(TimezoneDatabase::FindTimezone(timezone) == NULL) {
Missing " " before (
http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift@331
PS2, Line 331: The default is UTC,
This is a little unclear because the default value is "". I guess this means
that if the value is "", then that algorithm is used to decide?
http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py
File tests/custom_cluster/test_local_tz_conversion.py:
http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py@62
PS2, Line 62:
@CustomClusterTestSuite.with_args("--use_local_tz_for_unix_timestamp_conversions=true")
Can we combine this with test_utc_timestamp_functions() to avoid restarting the
cluster with the same options?
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 00:12:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2866/ -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 23:59:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/77/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 21:09:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/76/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 21:03:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2866/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 20:39:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#2). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. Examples: set timezone=UTC; set timezone="Europe/Budapest" Leading/trailing " characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has only effect if one of the following flags are true: -use_local_tz_for_unix_timestamp_conversions -convert_legacy_hive_parquet_utc_timestamps as there are no functions that do local time conversion by default. In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py 8 files changed, 88 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/2 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/77/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 20:31:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/76/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 20:28:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11064 Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. Examples: set timezone=UTC; set timezone="Europe/Budapest" Leading/trailing " characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has only effect if one of the following flags are true: -use_local_tz_for_unix_timestamp_conversions -convert_legacy_hive_parquet_utc_timestamps as there are no functions that do local time conversion by default. In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py 8 files changed, 90 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer
