Skip to content

Commit 8828a3b

Browse files
committed
Fix current_time handling for legacy timestamp semantics
Correct current_time behavior in legacy timestamp semantics when the session time zone’s current offset differs from its offset on 1970-01-01.
1 parent 814cdd6 commit 8828a3b

File tree

4 files changed

+19
-37
lines changed

4 files changed

+19
-37
lines changed

presto-main-base/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,12 @@ public static long currentTime(SqlFunctionProperties properties)
117117
// and we need to have UTC millis for packDateTimeWithZone
118118
long millis = UTC_CHRONOLOGY.millisOfDay().get(properties.getSessionStartTime());
119119

120-
if (!properties.isLegacyTimestamp()) {
121-
// However, those UTC millis are pointing to the correct UTC timestamp
122-
// Our TIME WITH TIME ZONE representation does use UTC 1970-01-01 representation
123-
// So we have to hack here in order to get valid representation
124-
// of TIME WITH TIME ZONE
125-
millis -= valueToSessionTimeZoneOffsetDiff(properties.getSessionStartTime(), getDateTimeZone(properties.getTimeZoneKey()));
126-
}
120+
// However, those UTC millis are pointing to the correct UTC timestamp
121+
// Our TIME WITH TIME ZONE representation does use UTC 1970-01-01 representation
122+
// So we have to hack here in order to get valid representation
123+
// of TIME WITH TIME ZONE
124+
millis -= valueToSessionTimeZoneOffsetDiff(properties.getSessionStartTime(), getDateTimeZone(properties.getTimeZoneKey()));
125+
127126
try {
128127
return packDateTimeWithZone(millis, properties.getTimeZoneKey());
129128
}

presto-main-base/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctions.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.joda.time.DateTime;
2121
import org.testng.annotations.Test;
2222

23-
import static com.facebook.presto.common.type.TimeWithTimeZoneType.TIME_WITH_TIME_ZONE;
2423
import static com.facebook.presto.common.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
2524
import static com.facebook.presto.common.type.VarcharType.createVarcharType;
2625

@@ -57,20 +56,6 @@ public void testLocalTime()
5756
}
5857
}
5958

60-
@Test
61-
public void testCurrentTime()
62-
{
63-
Session localSession = Session.builder(session)
64-
// we use Asia/Kathmandu here to test the difference in semantic change of current_time
65-
// between legacy and non-legacy timestamp
66-
.setTimeZoneKey(KATHMANDU_ZONE_KEY)
67-
.setStartTime(new DateTime(2017, 3, 1, 15, 45, 0, 0, KATHMANDU_ZONE).getMillis())
68-
.build();
69-
try (FunctionAssertions localAssertion = new FunctionAssertions(localSession)) {
70-
localAssertion.assertFunctionString("CURRENT_TIME", TIME_WITH_TIME_ZONE, "15:45:00.000 Asia/Kathmandu");
71-
}
72-
}
73-
7459
@Test
7560
public void testLocalTimestamp()
7661
{

presto-main-base/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,19 @@ private static long epochDaysInZone(TimeZoneKey timeZoneKey, long instant)
186186
return LocalDate.from(Instant.ofEpochMilli(instant).atZone(ZoneId.of(timeZoneKey.getId()))).toEpochDay();
187187
}
188188

189+
@Test
190+
public void testCurrentTime()
191+
{
192+
Session localSession = Session.builder(session)
193+
// we use Asia/Kathmandu here, as it has different zone offset on 2017-03-01 and on 1970-01-01
194+
.setTimeZoneKey(KATHMANDU_ZONE_KEY)
195+
.setStartTime(new DateTime(2017, 3, 1, 15, 45, 0, 0, KATHMANDU_ZONE).getMillis())
196+
.build();
197+
try (FunctionAssertions localAssertion = new FunctionAssertions(localSession)) {
198+
localAssertion.assertFunctionString("CURRENT_TIME", TIME_WITH_TIME_ZONE, "15:45:00.000 Asia/Kathmandu");
199+
}
200+
}
201+
189202
@Test
190203
public void testFromUnixTime()
191204
{

presto-main-base/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsLegacy.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.joda.time.DateTime;
2121
import org.testng.annotations.Test;
2222

23-
import static com.facebook.presto.common.type.TimeWithTimeZoneType.TIME_WITH_TIME_ZONE;
2423
import static com.facebook.presto.common.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
2524
import static com.facebook.presto.common.type.VarcharType.VARCHAR;
2625
import static com.facebook.presto.common.type.VarcharType.createVarcharType;
@@ -56,20 +55,6 @@ public void testLocalTime()
5655
}
5756
}
5857

59-
@Test
60-
public void testCurrentTime()
61-
{
62-
Session localSession = Session.builder(session)
63-
// we use Asia/Kathmandu here to test the difference in semantic change of current_time
64-
// between legacy and non-legacy timestamp
65-
.setTimeZoneKey(KATHMANDU_ZONE_KEY)
66-
.setStartTime(new DateTime(2017, 3, 1, 15, 45, 0, 0, KATHMANDU_ZONE).getMillis())
67-
.build();
68-
try (FunctionAssertions localAssertion = new FunctionAssertions(localSession)) {
69-
localAssertion.assertFunctionString("CURRENT_TIME", TIME_WITH_TIME_ZONE, "15:30:00.000 Asia/Kathmandu");
70-
}
71-
}
72-
7358
@Test
7459
public void testLocalTimestamp()
7560
{

0 commit comments

Comments
 (0)