Change time zone used in tests to better test date/time/zone/JVM-related corner cases

[findepi]

All tests are run with Asia/Kathmandu time zone. I think it was chosen because:

1. it's zone offset has changed since 1970
2. it's zone offset is not whole hours (was it the case?)

These are nice properties, but there are other nice properties a time zone could have:

3. DST (breaks so many things)
4. having DST change at midnight (breaks all those code paths that represent date as local time at midnight)

Since it's not possible to change JVM default zone for just a single test, using Asia/Kathmandu makes it not possible to test functionalities around DST changes. For example, reading TIMESTAMP values using JDBC ResultSet.getTimestamp() cannot succeed when TIMESTAMP value to be returned never occurred in JVM time zone, was skipped (DST change forward).

For these reasons, I'd like to change time zone used in tests to one having DST. Unfortunately there isn't a zone that has all 4 properties on regular basis. I wish we could create a new one, just for programs testing..

I think we should choose one of these:

• Pacific/Chatham (has properties: 1, 2, 3 and partially 4)

  • offset Jan 1970: +12:45, offset Jan 2018: +13:45
  • DST on Jan '18: true
  • doesn't have DST change at midnight... but had twice in the past, most recently on Tuesday, 1 January 1946, 00:00:00 clocks were turned forward 0:30 hours
    • maybe one DST change at midnight is actually enough. I'd prefer a year >1970 however

• Pacific/Apia (has properties: 1, 1, 1, 1, 1, 1, 1, 2-, 3, 4)

  • offset Jan 1970: -11:00, offset Jan 2018: +14:00 (25h difference, the only one such: on Friday, 30 December 2011, 00:00:00 clocks were turned forward 24 hours)
  • DST on Jan '18: true
  • .. but doesn't have not-whole-hours-offset (i'm unsure how important is that)
  • doesn't have DST change at midnight, but had several times in the past, most recently on Sunday, 26 September 2010, 00:00:00 clocks were turned forward 1 hour
    • one DST change at midnight is actually enough

I am aware changing time zone will require changing expected values in tests, whenever TIMESTAMP is touched, due to #7122.

cc @losipiuk @martint @electrum @haozhun

---

• (Change JVM time zone in tests to better test corner cases #10128) tests' JVM zone → America/Bahia_Banderas
• product tests' zone(s) → ?
• default session zone in tests (TestingSession) → Europe/Vilnius

[dain]

2. it's zone offset is not whole hours (was it the case?) 

Yes... It makes it easy to detect bugs/error because the printed time is so different from the current time.

I'm ok with either choice. For Pacific/Chatham I agree that the < 1970 is a bit annoying.... I'd lean towards Pacific/Apia, but not super strong preference.

[ArturGajowy]

How about choosing one of the hard timezones randomly on test start? 😈

[kokosing]

Maybe we could have couple test suites where each would leverage different TZ (different JVM fork)? It is a bit annoying infrastructure but that would cover all the corner cases.

[findepi]

@kokosing with this approach, what would become possible to test, what isn't possible if we go with e.g. Pacific/Apia?

[kokosing]

I meant that we could stay with Asia/Kathmandu for regular tests and use Pacific/Apia for tests around DST. I am not sure if it is worth to stay with Asia/Kathmandu just to keep not whole hours zone offset.

[findepi]

Actually there are two time zone definitions used in tests:

• pom.xml's <air.test.timezone>Asia/Katmandu</air.test.timezone> -- this controls JVM time zone, which impacts how java.sql.{Date,Time,Timestamp} behave (and also what values are representable)
• TestingSession's .setTimeZoneKey(UTC_KEY) -- this controls session time zone, and this impacts Presto behavior (most importantly: TIMESTAMP behaviour does not match sql standard #7122). With session in UTC zone, some problems go unnoticed.

I was initially thinking about the first one, but now I think we should change both and to some different zones.

[findepi]

Also, the proposed zones have local time change at midnight only forward. Forward change is more important as it affects representability of date-time values in java.sql.{Date,Timestamp} classes.
If, however, we wanted a zone that has DST change both forward and backward, we need to give up property #2 (zone offset is not whole hours)... Then America/Asuncion would be a candidate (properties 1, 3, 4+).

The bad news is that there is other interesting property:

5. zone offset change on Jan 1, 1970: this affects time representability for java.sql.Time as well as Presto's TIME WITH TIME ZONE (see this comment in "TIMESTAMP official").

There are 3 zones that have properties (1, 3, 4-, 5):

            America/Bahia_Banderas:  offset Jan 1970: -08:00, offset Jan 2018: -06:00,  DST Jan '18: false, DST Jul '18:  true, shift on 1970-01-01: true
                  America/Mazatlan:  offset Jan 1970: -08:00, offset Jan 2018: -07:00,  DST Jan '18: false, DST Jul '18:  true, shift on 1970-01-01: true
                    Mexico/BajaSur:  offset Jan 1970: -08:00, offset Jan 2018: -07:00,  DST Jan '18: false, DST Jul '18:  true, shift on 1970-01-01: true

[findepi]

My apologies. I intended this to be well-prepared proposal before posting this issue, but it turned into thinking aloud...

There simply isn't a time zone that is a perfect fit.

[findepi]

Doing this change may help with testing in #7122.

[findepi]

@dain, I singled out America/Bahia_Banderas as my candidate. Zone offset changes on Jan 1, 1970 are invaluable.
For now, I use this zone in #10114 in postgres and mysql modules as tests JVM zone, but I think we could apply it universally.
I didn't make my mind yet as to what should the default session zone in tests be.

[haozhun]

@findepi

I wasn't involved with Presto when the decision was made. But one of the very important reasons here (that I remember @dain talking about) is that its offset is positive. Given Presto team works in a negative zone, we had bugs that only affect users in positive zones. And that went unnoticed.

I don't care that much about whole-hour. Assuming basic competitiveness of the code, I can't think of a way non-whole-hour zone can break.

[findepi]

@haozhun let's use a positive offset zone for default session zone in tests (TestingSession). Anyway, we shouldn't have zones: (1) tests JVM zone and (2) tests' default session zone both default to the same (or similar) things. What do you think?

[findepi]

i added checkboxes to the issue description

[findepi]

I am updating #10128 to cover product tests too (switching them to America/Bahia_Banderas).
Once we don't use Kathmandu zone for JVM zone any more, i think we could use it for default session zone (TestingSession). I previously proposed Vilnius zone for this, but I forgot why I did so :)

[findepi]

Once we don't use Kathmandu zone for JVM zone any more, i think we could use it for default session zone (TestingSession). I previously proposed Vilnius zone for this, but I forgot why I did so :)

I remember now. Kathmandu doesn't have DST.

[findepi]

As explained in #10940 (comment), we cannot use a zone with "property 5" (forward offset change at 1970-01-01 00:00:00 local time) as product tests's zone.
Thus, we are going to have 2 different JVM zones:

• one for regular tests (America/Bahia_Banderas, Change JVM time zone in tests to better test corner cases #10128)
• and one for product tests (all containers, including remote servers, presto and tests' VM)

I propose to use Pacific/Apia (having properties 1++,2-,3,4) for product tests, instead of Kathmandu used today.