-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[IOTDB-17433] Fix timezone offset bug in DateTimeUtils #17439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -558,14 +558,14 @@ public static long convertTimestampOrDatetimeStrToLongWithDefaultZone(String tim | |||||||||||||||||||||||||||||||||||
| public static long convertDatetimeStrToLong(String str, ZoneId zoneId) { | ||||||||||||||||||||||||||||||||||||
| return convertDatetimeStrToLong( | ||||||||||||||||||||||||||||||||||||
| str, | ||||||||||||||||||||||||||||||||||||
| toZoneOffset(zoneId), | ||||||||||||||||||||||||||||||||||||
| toZoneOffset(str, zoneId), | ||||||||||||||||||||||||||||||||||||
| 0, | ||||||||||||||||||||||||||||||||||||
| CommonDescriptor.getInstance().getConfig().getTimestampPrecision()); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public static long convertDatetimeStrToLong( | ||||||||||||||||||||||||||||||||||||
| String str, ZoneId zoneId, String timestampPrecision) { | ||||||||||||||||||||||||||||||||||||
| return convertDatetimeStrToLong(str, toZoneOffset(zoneId), 0, timestampPrecision); | ||||||||||||||||||||||||||||||||||||
| return convertDatetimeStrToLong(str, toZoneOffset(str, zoneId), 0, timestampPrecision); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public static long getInstantWithPrecision(String str, String timestampPrecision) { | ||||||||||||||||||||||||||||||||||||
|
|
@@ -821,8 +821,22 @@ public static ZonedDateTime convertToZonedDateTime(long timestamp, ZoneId zoneId | |||||||||||||||||||||||||||||||||||
| return ZonedDateTime.ofInstant(Instant.ofEpochMilli(timestamp), zoneId); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public static ZoneOffset toZoneOffset(ZoneId zoneId) { | ||||||||||||||||||||||||||||||||||||
| return zoneId.getRules().getOffset(Instant.now()); | ||||||||||||||||||||||||||||||||||||
| /** Converts string to ZoneOffset. Truncates seconds for HH:mm database compatibility. */ | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| /** Converts string to ZoneOffset. Truncates seconds for HH:mm database compatibility. */ | |
| /** | |
| * Converts a date-time string to a {@link ZoneOffset}. | |
| * | |
| * <p>If the input ends with {@code Z}, this method returns {@link ZoneOffset#UTC}. If the | |
| * input contains an explicit {@code \u00b1HH:mm} offset, that offset is returned directly. | |
| * Otherwise, the input is interpreted as a local date-time in the supplied {@code zoneId}, and | |
| * the corresponding zone offset is derived from that local date-time. | |
| * | |
| * <p>For database {@code HH:mm} compatibility, this method truncates the resulting offset to | |
| * minute precision by discarding any offset seconds. It does not truncate the seconds field of | |
| * the input date-time. | |
| * | |
| * <p>When deriving the offset from a local date-time, DST transitions are resolved using {@link | |
| * java.time.zone.ZoneRules#getOffset(LocalDateTime)}: for overlaps, the earlier offset is | |
| * returned; for gaps, the offset before the transition is returned. | |
| */ |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,12 +27,14 @@ | |||||
| import org.junit.Ignore; | ||||||
| import org.junit.Test; | ||||||
|
|
||||||
| import java.time.DateTimeException; | ||||||
| import java.time.Instant; | ||||||
| import java.time.ZoneId; | ||||||
| import java.time.ZoneOffset; | ||||||
| import java.time.ZonedDateTime; | ||||||
| import java.util.TimeZone; | ||||||
|
|
||||||
| import static org.junit.Assert.assertEquals; | ||||||
| import static org.junit.Assert.assertThrows; | ||||||
| import static org.junit.Assert.fail; | ||||||
|
|
||||||
| public class DateTimeUtilsTest { | ||||||
|
|
@@ -47,7 +49,7 @@ public class DateTimeUtilsTest { | |||||
| /** Test convertDatetimeStrToLong() method with different time precision. */ | ||||||
| @Test | ||||||
| public void convertDatetimeStrToLongTest1() { | ||||||
| zoneOffset = ZonedDateTime.now().getOffset(); | ||||||
| zoneOffset = Instant.ofEpochMilli(timestamp).atZone(ZoneId.systemDefault()).getOffset(); | ||||||
| zoneId = ZoneId.systemDefault(); | ||||||
| if (zoneOffset.toString().equals("Z")) { | ||||||
| delta = 8 * 3600000; | ||||||
|
|
@@ -338,4 +340,95 @@ public void testConstructTimeDuration() { | |||||
| timeDuration = DateTimeUtils.constructTimeDuration("10000000000ms"); | ||||||
| Assert.assertEquals(10000000000L, timeDuration.nonMonthDuration); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testToZoneOffsetForWinterTime() { | ||||||
| ZoneId zoneId = ZoneId.of("Europe/Warsaw"); | ||||||
| ZoneOffset offset = DateTimeUtils.toZoneOffset("2024-01-15 12:00:00", zoneId); | ||||||
| Assert.assertEquals(ZoneOffset.ofHours(1), offset); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testToZoneOffsetForSummerTime() { | ||||||
| ZoneId zoneId = ZoneId.of("Europe/Warsaw"); | ||||||
| ZoneOffset offset = DateTimeUtils.toZoneOffset("2024-06-15 12:00:00", zoneId); | ||||||
| Assert.assertEquals(ZoneOffset.ofHours(2), offset); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testToZoneOffsetJustBeforeSpringDST() { | ||||||
| ZoneId zoneId = ZoneId.of("Europe/Warsaw"); | ||||||
| ZoneOffset offset = DateTimeUtils.toZoneOffset("2024-03-31 02:00:00", zoneId); | ||||||
|
||||||
| ZoneOffset offset = DateTimeUtils.toZoneOffset("2024-03-31 02:00:00", zoneId); | |
| ZoneOffset offset = DateTimeUtils.toZoneOffset("2024-03-31 01:59:59", zoneId); |
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testToZoneOffsetDuringSpringDSTGap asserts an offset for 2024-03-31 02:30:00 in Europe/Warsaw, but this local time does not exist due to the DST spring-forward gap. The current expectation relies on how gaps are resolved by ZoneRules and can become brittle across JDK/tzdb changes. It would be more robust to either (a) use ZoneRules#getValidOffsets/transition logic in production code and assert that documented behavior here, or (b) change the test to expect a failure/explicit resolution rather than asserting a specific offset for a nonexistent local time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convertDatetimeStrToLong(String, ZoneId)now callstoZoneOffset(str, zoneId), andtoZoneOffsetreparses the same string viaconvertDatetimeStrToLong(str, ZoneOffset.UTC, ...)beforeconvertDatetimeStrToLongparses it again with the resolved offset. This introduces an extra full parse for datetime strings without an explicit offset, which could be a hot-path performance regression. If this method is performance-sensitive, consider extracting the local date-time once (or otherwise avoiding the double parse) while still supporting the various input formats.