[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-22 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has abandoned this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-22 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 5:

As https://gerrit.cloudera.org/#/c/9986/ was merged, this change no longer 
makes sense on the master branch, so I am abandoning it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Fri, 22 Jun 2018 14:00:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-04 Thread Jim Apple (Code Review) 
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 5: Code-Review-1

I'm a -1 on this until we have community decision about version numbers.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Mon, 04 Jun 2018 18:32:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-04 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10486/4/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/10486/4/be/src/exprs/timezone_db.cc@628
PS4, Line 628: \"SystemV/AST4\",\"AST\",\"Atlantic Standard 
Time\",\"\",\"\",\"-04:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/AST4ADT\",\"AST\",\"Atlantic Standard 
Time\",\"ADT\",\"Atlantic Daylight 
Time\",\"-04:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/CST6\",\"CST\",\"Central Standard 
Time\",\"\",\"\",\"-06:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/CST6CDT\",\"CST\",\"Central Standard 
Time\",\"CDT\",\"Central Daylight 
Time\",\"-06:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/EST5\",\"EST\",\"Eastern Standard 
Time\",\"\",\"\",\"-05:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/EST5EDT\",\"EST\",\"Eastern Standard 
Time\",\"EDT\",\"Eastern Daylight 
Time\",\"-05:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/HST10\",\"HST\",\"Hawaii Standard 
Time\",\"\",\"\",\"-10:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/MST7\",\"MST\",\"Mountain Standard 
Time\",\"\",\"\",\"-07:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/MST7MDT\",\"MST\",\"Mountain Standard 
Time\",\"MDT\",\"Mountain Daylight 
Time\",\"-07:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/PST8\",\"PST\",\"Pacific Standard 
Time\",\"\",\"\",\"-08:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/PST8PDT\",\"PST\",\"Pacific Standard 
Time\",\"PDT\",\"Pacific Daylight 
Time\",\"-08:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/YST9\",\"AKST\",\"Alaska Standard 
Time\",\"\",\"\",\"-09:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/YST9YDT\",\"AKST\",\"Alaska Standard 
Time\",\"AKDT\",\"Alaska Daylight 
Time\",\"-09:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
> I think these lines should be removed too.
Done


http://gerrit.cloudera.org:8080/#/c/10486/4/testdata/data/timezoneverification.csv
File testdata/data/timezoneverification.csv:

http://gerrit.cloudera.org:8080/#/c/10486/4/testdata/data/timezoneverification.csv@806
PS4, Line 806: SystemV/AST4,2015-06-20 15:00:00,2015-06-20 11:00:00
 : SystemV/AST4ADT,2015-06-20 15:00:00,2015-06-20 12:00:00
 : SystemV/AST4ADT,2014-12-20 15:00:00,2014-12-20 11:00:00
 : SystemV/CST6,2015-06-20 15:00:00,2015-06-20 09:00:00
 : SystemV/CST6CDT,2015-06-20 15:00:00,2015-06-20 10:00:00
 : SystemV/CST6CDT,2014-12-20 15:00:00,2014-12-20 09:00:00
 : SystemV/EST5,2015-06-20 15:00:00,2015-06-20 10:00:00
 : SystemV/EST5EDT,2015-06-20 15:00:00,2015-06-20 11:00:00
 : SystemV/EST5EDT,2014-12-20 15:00:00,2014-12-20 10:00:00
 : SystemV/HST10,2015-06-20 15:00:00,2015-06-20 05:00:00
 : SystemV/MST7,2015-06-20 15:00:00,2015-06-20 08:00:00
 : SystemV/MST7MDT,2015-06-20 15:00:00,2015-06-20 09:00:00
 : SystemV/MST7MDT,2014-12-20 15:00:00,2014-12-20 08:00:00
 : SystemV/PST8,2015-06-20 15:00:00,2015-06-20 07:00:00
 : SystemV/PST8PDT,2015-06-20 15:00:00,2015-06-20 08:00:00
 : SystemV/PST8PDT,2014-12-20 15:00:00,2014-12-20 07:00:00
 : SystemV/YST9,2015-06-20 15:00:00,2015-06-20 06:00:00
 : SystemV/YST9YDT,2015-06-20 15:00:00,2015-06-20 07:00:00
 : SystemV/YST9YDT,2014-12-20 15:00:00,2014-12-20 06:00:00
> These should be removed too, see my previous comment.
Done - note that these tests were not removed from 
https://gerrit.cloudera.org/#/c/9986/10/testdata/data/timezoneverification.csv



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Mon, 04 Jun 2018 17:58:53 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-04 Thread Csaba Ringhofer (Code Review) 
Hello Jim Apple, Attila Jeges,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10486

to look at the new patch set (#5).

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it.

Testing:
- a few timezones are removed from timezoneverification.csv
- added test for invalid timezone names

Cherry-picks: not for 2.x.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M testdata/data/timezoneverification.csv
M tests/query_test/test_timezones.py
4 files changed, 17 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10486/5
--
To view, visit http://gerrit.cloudera.org:8080/10486
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-02 Thread Attila Jeges (Code Review) 
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10486/4/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/10486/4/be/src/exprs/timezone_db.cc@628
PS4, Line 628: \"SystemV/AST4\",\"AST\",\"Atlantic Standard 
Time\",\"\",\"\",\"-04:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/AST4ADT\",\"AST\",\"Atlantic Standard 
Time\",\"ADT\",\"Atlantic Daylight 
Time\",\"-04:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/CST6\",\"CST\",\"Central Standard 
Time\",\"\",\"\",\"-06:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/CST6CDT\",\"CST\",\"Central Standard 
Time\",\"CDT\",\"Central Daylight 
Time\",\"-06:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/EST5\",\"EST\",\"Eastern Standard 
Time\",\"\",\"\",\"-05:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/EST5EDT\",\"EST\",\"Eastern Standard 
Time\",\"EDT\",\"Eastern Daylight 
Time\",\"-05:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/HST10\",\"HST\",\"Hawaii Standard 
Time\",\"\",\"\",\"-10:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/MST7\",\"MST\",\"Mountain Standard 
Time\",\"\",\"\",\"-07:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/MST7MDT\",\"MST\",\"Mountain Standard 
Time\",\"MDT\",\"Mountain Daylight 
Time\",\"-07:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/PST8\",\"PST\",\"Pacific Standard 
Time\",\"\",\"\",\"-08:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/PST8PDT\",\"PST\",\"Pacific Standard 
Time\",\"PDT\",\"Pacific Daylight 
Time\",\"-08:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
 : \"SystemV/YST9\",\"AKST\",\"Alaska Standard 
Time\",\"\",\"\",\"-09:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\"\n\
 : \"SystemV/YST9YDT\",\"AKST\",\"Alaska Standard 
Time\",\"AKDT\",\"Alaska Daylight 
Time\",\"-09:00:00\",\"+01:00:00\",\"-1;0;4\",\"+02:00:00\",\"-1;0;10\",\"+02:00:00\"\n\
I think these lines should be removed too.

These are old timezones, kept only for legacy reasons. Although the 
corresponding rules are part of the IANA timezone data, they are commented out, 
so they won't be compiled by default.


http://gerrit.cloudera.org:8080/#/c/10486/4/testdata/data/timezoneverification.csv
File testdata/data/timezoneverification.csv:

http://gerrit.cloudera.org:8080/#/c/10486/4/testdata/data/timezoneverification.csv@806
PS4, Line 806: SystemV/AST4,2015-06-20 15:00:00,2015-06-20 11:00:00
 : SystemV/AST4ADT,2015-06-20 15:00:00,2015-06-20 12:00:00
 : SystemV/AST4ADT,2014-12-20 15:00:00,2014-12-20 11:00:00
 : SystemV/CST6,2015-06-20 15:00:00,2015-06-20 09:00:00
 : SystemV/CST6CDT,2015-06-20 15:00:00,2015-06-20 10:00:00
 : SystemV/CST6CDT,2014-12-20 15:00:00,2014-12-20 09:00:00
 : SystemV/EST5,2015-06-20 15:00:00,2015-06-20 10:00:00
 : SystemV/EST5EDT,2015-06-20 15:00:00,2015-06-20 11:00:00
 : SystemV/EST5EDT,2014-12-20 15:00:00,2014-12-20 10:00:00
 : SystemV/HST10,2015-06-20 15:00:00,2015-06-20 05:00:00
 : SystemV/MST7,2015-06-20 15:00:00,2015-06-20 08:00:00
 : SystemV/MST7MDT,2015-06-20 15:00:00,2015-06-20 09:00:00
 : SystemV/MST7MDT,2014-12-20 15:00:00,2014-12-20 08:00:00
 : SystemV/PST8,2015-06-20 15:00:00,2015-06-20 07:00:00
 : SystemV/PST8PDT,2015-06-20 15:00:00,2015-06-20 08:00:00
 : SystemV/PST8PDT,2014-12-20 15:00:00,2014-12-20 07:00:00
 : SystemV/YST9,2015-06-20 15:00:00,2015-06-20 06:00:00
 : SystemV/YST9YDT,2015-06-20 15:00:00,2015-06-20 07:00:00
 : SystemV/YST9YDT,2014-12-20 15:00:00,2014-12-20 06:00:00
These should be removed too, see my previous comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Sat, 02 Jun 2018 07:35:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-06-01 Thread Csaba Ringhofer (Code Review) 
Hello Jim Apple, Attila Jeges,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10486

to look at the new patch set (#4).

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it.

Testing:
- a few timezones are removed from timezoneverification.csv
- added test for invalid timezone names

Cherry-picks: not for 2.x.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M testdata/data/timezoneverification.csv
M tests/query_test/test_timezones.py
4 files changed, 17 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10486/4
--
To view, visit http://gerrit.cloudera.org:8080/10486
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Attila Jeges (Code Review) 
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Thu, 24 May 2018 17:33:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Jim Apple (Code Review) 
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 3:

> > Is this a breaking change? We normally hold on braking changes
 > > until we bump the major version.
 > >
 > > Is IMPALA-3307 going to go in on a minor release? Is it a
 > breaking
 > > change?
 >
 > Yes, this and IMPALA-3307 are breaking changes. This change should
 > remove all timezones that won't be supported after IMPALA-3307.
 > IMPALA-3307 will introduce some changes related to timezones that
 > were changed during history, but I consider those changes to be
 > fixes rather than breaking changes.

Thanks for the clarification. Let's talk about this on dev@, which has more 
visibility, since intentionally introducing a breaking change between major 
versinos is usually something Impala has avoided.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Thu, 24 May 2018 16:39:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

> Is this a breaking change? We normally hold on braking changes
 > until we bump the major version.
 > 
 > Is IMPALA-3307 going to go in on a minor release? Is it a breaking
 > change?

Yes, this and IMPALA-3307 are breaking changes. This change should remove all 
timezones that won't be supported after IMPALA-3307. IMPALA-3307 will introduce 
some changes related to timezones that were changed during history, but I 
consider those changes to be fixes rather than breaking changes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Thu, 24 May 2018 14:31:45 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG@29
PS1, Line 29:
> I think you should add "Cherry-picks: not for 2.x." line.
Done


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h
File be/src/exprs/timezone_db.h:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55
PS1, Line 55: map
> const?
This map turned out to be unnecessary.


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc@68
PS1, Line 68: {"AEST", "Australia/Sydney"},
: {"CDT", "America/Chicago"},
: {"CEST", "CET"},
: {"EDT", "EST5EDT"},
: {"ICT", "Asia/Ho_Chi_Minh"},
: {"KST", "Asia/Seoul"},
: {"MDT", "MST7MDT"},
: {"PHT", "Asia/Manila"},
: {"PDT", "America/Los_Angeles"}
> Are these time-zone abbreviations accepted by Hive? If not, I think we shou
Done - I have checked TimezoneDatabase::TIMEZONE_DATABASE_STR, and as all the 
Java abbreviations are already there, I have removed this map.


http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv
File testdata/data/timezoneverification.csv:

http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv@716
PS1, Line 716: JST
> Please make sure hat all the Java-supported time-zone abbreviations are tes
I have checked, all of them are included here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Thu, 24 May 2018 14:27:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Csaba Ringhofer (Code Review) 
Hello Jim Apple, Attila Jeges,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10486

to look at the new patch set (#3).

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it.

Testing:
Timestamp aliases were largely untested, so only a few changes were
needed - these were copied from the review for IMPALA-3307,
gerrit.cloudera.org/#/c/9986/ It may be useful to add more tests,
e.g. to check that using dropped aliases return NULL and lead to
warning, but I think that these kind of changes should be done first
in the review of IMPALA-3307, and synced here afterwards.

Cherry-picks: not for 2.x.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M testdata/data/timezoneverification.csv
3 files changed, 2 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10486/3
--
To view, visit http://gerrit.cloudera.org:8080/10486
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-24 Thread Csaba Ringhofer (Code Review) 
Hello Jim Apple, Attila Jeges,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10486

to look at the new patch set (#2).

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it.

Testing:
Timestamp aliases were largely untested, so only a few changes were
needed - these were copied from the review for IMPALA-3307,
gerrit.cloudera.org/#/c/9986/ It may be useful to add more tests,
e.g. to check that using dropped aliases return NULL and lead to
warning, but I think that these kind of changes should be done first
in the review of IMPALA-3307, and synced here afterwards.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M testdata/data/timezoneverification.csv
3 files changed, 2 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10486/2
--
To view, visit http://gerrit.cloudera.org:8080/10486
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Jim Apple (Code Review) 
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

(1 comment)

Is this a breaking change? We normally hold on braking changes until we bump 
the major version.

Is IMPALA-3307 going to go in on a minor release? Is it a breaking change?

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h
File be/src/exprs/timezone_db.h:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55
PS1, Line 55: map
> Maybe use unordered_map instead ? Probably wouldn't make much of a differen
const?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Wed, 23 May 2018 18:30:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Attila Jeges (Code Review) 
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG@29
PS1, Line 29:
I think you should add "Cherry-picks: not for 2.x." line.


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h
File be/src/exprs/timezone_db.h:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55
PS1, Line 55: map
Maybe use unordered_map instead ? Probably wouldn't make much of a difference 
though.


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc@68
PS1, Line 68: {"AEST", "Australia/Sydney"},
: {"CDT", "America/Chicago"},
: {"CEST", "CET"},
: {"EDT", "EST5EDT"},
: {"ICT", "Asia/Ho_Chi_Minh"},
: {"KST", "Asia/Seoul"},
: {"MDT", "MST7MDT"},
: {"PHT", "Asia/Manila"},
: {"PDT", "America/Los_Angeles"}
Are these time-zone abbreviations accepted by Hive? If not, I think we should 
remove them.


http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv
File testdata/data/timezoneverification.csv:

http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv@716
PS1, Line 716: JST
Please make sure hat all the Java-supported time-zone abbreviations are tested 
here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Comment-Date: Wed, 23 May 2018 17:53:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10486


Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it. It should be much faster than
the previous solution for timezone aliases, while slightly slower
for canonical names.

Testing:
Timestamp aliases were largely untested, so only a few changes were
needed - these were copied from the review for IMPALA-3307,
gerrit.cloudera.org/#/c/9986/ It may be useful to add more tests,
e.g. to check that using dropped aliases return NULL and lead to
warning, but I think that these kind of changes should be done first
in the review of IMPALA-3307, and synced here afterwards.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M be/src/exprs/timezone_db.h
M testdata/data/timezoneverification.csv
4 files changed, 46 insertions(+), 29 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10486/1
--
To view, visit http://gerrit.cloudera.org:8080/10486
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer