You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2017/09/26 15:43:58 UTC
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8139
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
Moscow timezone is handled differrently than other timezones,
and it was possible to cause unhandled boost exception by
trying to convert "dateless" timestamps like "10:00:00".
These timestamps cannot be handled by timestamp conversion
generally, because daylight saving time logic needs month
and day to work correctly. The condition HasDateOrTime()
incorrectly suggested that these timestamps can be handled,
so I made the condition stricter.
Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
---
M be/src/exprs/timestamp-functions.cc
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8139/1
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py
File tests/query_test/test_timezones.py:
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py@45
PS3, Line 45: # timestamp conversions of "dateless" times should return null (and not crash,
As discussed in person, please move the tests to test-exprs.cc.
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:19:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py
File tests/query_test/test_timezones.py:
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py@45
PS3, Line 45:
> As discussed in person, please move the tests to test-exprs.cc.
Done
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 11:27:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 2:
> Thanks for fixing this. Can you add some tests?
I have added a new check an end-to-end tests.
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Sep 2017 15:05:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 03:01:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1322/
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Oct 2017 23:15:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Oct 2017 23:15:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
Moscow timezone is handled differrently than other timezones,
and it was possible to cause unhandled boost exception by
trying to convert "dateless" timestamps like "10:00:00".
These timestamps cannot be handled by timestamp conversion
generally, because daylight saving time logic needs month
and day to work correctly. The condition HasDateOrTime()
incorrectly suggested that these timestamps can be handled,
so I made the condition stricter.
Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Reviewed-on: http://gerrit.cloudera.org:8080/8139
Tested-by: Impala Public Jenkins
Reviewed-by: Lars Volker <lv...@cloudera.com>
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions.cc
2 files changed, 7 insertions(+), 2 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Lars Volker: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 4: Code-Review+1
Will let Lars finish
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 21:30:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8139
to look at the new patch set (#4).
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
Moscow timezone is handled differrently than other timezones,
and it was possible to cause unhandled boost exception by
trying to convert "dateless" timestamps like "10:00:00".
These timestamps cannot be handled by timestamp conversion
generally, because daylight saving time logic needs month
and day to work correctly. The condition HasDateOrTime()
incorrectly suggested that these timestamps can be handled,
so I made the condition stricter.
Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions.cc
2 files changed, 7 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8139/4
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8139
to look at the new patch set (#3).
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
Moscow timezone is handled differrently than other timezones,
and it was possible to cause unhandled boost exception by
trying to convert "dateless" timestamps like "10:00:00".
These timestamps cannot be handled by timestamp conversion
generally, because daylight saving time logic needs month
and day to work correctly. The condition HasDateOrTime()
incorrectly suggested that these timestamps can be handled,
so I made the condition stricter.
Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
---
M be/src/exprs/timestamp-functions.cc
M tests/query_test/test_timezones.py
2 files changed, 9 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8139/3
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 1:
Thanks for fixing this. Can you add some tests?
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Sep 2017 16:16:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 5: Code-Review+2
Carrying my +2 after the rebase.
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Oct 2017 16:39:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8139
to look at the new patch set (#2).
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
Moscow timezone is handled differrently than other timezones,
and it was possible to cause unhandled boost exception by
trying to convert "dateless" timestamps like "10:00:00".
These timestamps cannot be handled by timestamp conversion
generally, because daylight saving time logic needs month
and day to work correctly. The condition HasDateOrTime()
incorrectly suggested that these timestamps can be handled,
so I made the condition stricter.
Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
---
M be/src/exprs/timestamp-functions.cc
M tests/query_test/test_timezones.py
2 files changed, 9 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8139/2
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-5983: Fix crash in to/from utc timestamp("10:00:00", 'MSK')
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8139 )
Change subject: IMPALA-5983: Fix crash in to/from_utc_timestamp("10:00:00", 'MSK')
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py
File tests/query_test/test_timezones.py:
http://gerrit.cloudera.org:8080/#/c/8139/3/tests/query_test/test_timezones.py@45
PS3, Line 45: # timestamp conversions of "dateless" times should return null (and not crash,
> As discussed in person, please move the tests to test-exprs.cc.
expr-test.cc
--
To view, visit http://gerrit.cloudera.org:8080/8139
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I592389333a16a15a680beed389e2702dfc16fe1d
Gerrit-Change-Number: 8139
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Sep 2017 00:46:39 +0000
Gerrit-HasComments: Yes