From e75b192f46fe4d27b2dcf5ce42bfbe2d1da5d320 Mon Sep 17 00:00:00 2001 From: Chip Kent <5250374+chipkent@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:39:19 -0400 Subject: [PATCH] fix!: Fix a TimeRange off-by-one bug in nanosecond calculation (#5648) Fix a TimeRange off-by-one bug in nanosecond calculation. Resolves #5646 BREAKING CHANGE: The length of a business day was being incorrectly computed by 1 ns. To keep the business day length the same for existing calendars, the close of a business period has been changed from inclusive to exclusive. This default behavior can be overridden by using `true` when specifying time ranges. This will include the closing time for the range and will result in a business period that is 1 ns longer. Business calendars using the legacy format must be upgraded to the current format to use `true`. --- .../calendar/BusinessCalendarXMLParser.java | 15 ++++- .../io/deephaven/time/calendar/TimeRange.java | 2 +- .../TestBusinessCalendarXMLParser.java | 3 + .../time/calendar/TestCalendarDay.java | 60 +++++++++---------- .../time/calendar/TestCalendars.java | 10 ++++ .../time/calendar/TestTimeRange.java | 23 +++++-- .../src/test/resources/PARSER-TEST.calendar | 2 +- .../src/main/resources/calendar/UTC.calendar | 2 +- 8 files changed, 77 insertions(+), 40 deletions(-) diff --git a/engine/time/src/main/java/io/deephaven/time/calendar/BusinessCalendarXMLParser.java b/engine/time/src/main/java/io/deephaven/time/calendar/BusinessCalendarXMLParser.java index 6f2c0432b9b..7c430b43cc6 100644 --- a/engine/time/src/main/java/io/deephaven/time/calendar/BusinessCalendarXMLParser.java +++ b/engine/time/src/main/java/io/deephaven/time/calendar/BusinessCalendarXMLParser.java @@ -35,7 +35,12 @@ * New York Stock Exchange Calendar * America/New_York * - * 09:3016:00 + * + * 09:30 + * 16:00 + * + * true + * * Saturday * Sunday * @@ -51,6 +56,8 @@ * * 09:30 * 13:00 + * + * true * * * @@ -254,6 +261,7 @@ private static TimeRange[] parseBusinessRanges(final List bu for (int i = 0; i < businessRanges.size(); i++) { final String openTxt = getText(getRequiredChild(businessRanges.get(i), "open")); final String closeTxt = getText(getRequiredChild(businessRanges.get(i), "close")); + final String includeCloseTxt = getText(businessRanges.get(i).getChild("includeClose")); if (closeTxt.startsWith("24:00")) { throw new RuntimeException("Close time (" + closeTxt @@ -262,7 +270,8 @@ private static TimeRange[] parseBusinessRanges(final List bu final LocalTime open = DateTimeUtils.parseLocalTime(openTxt); final LocalTime close = DateTimeUtils.parseLocalTime(closeTxt); - rst[i] = new TimeRange<>(open, close, true); + final boolean inclusiveEnd = Boolean.parseBoolean(includeCloseTxt); // defaults to false + rst[i] = new TimeRange<>(open, close, inclusiveEnd); } return rst; @@ -282,7 +291,7 @@ private static TimeRange[] parseBusinessRangesLegacy(final List(open, close, true); + rst[i] = new TimeRange<>(open, close, false); } else { throw new IllegalArgumentException("Can not parse business periods; open/close = " + br.getText()); } diff --git a/engine/time/src/main/java/io/deephaven/time/calendar/TimeRange.java b/engine/time/src/main/java/io/deephaven/time/calendar/TimeRange.java index e412673eb9d..837f8a23fca 100644 --- a/engine/time/src/main/java/io/deephaven/time/calendar/TimeRange.java +++ b/engine/time/src/main/java/io/deephaven/time/calendar/TimeRange.java @@ -81,7 +81,7 @@ public boolean isInclusiveEnd() { * @return length of the range in nanoseconds */ public long nanos() { - return start.until(end, ChronoUnit.NANOS) - (inclusiveEnd ? 0 : 1); + return start.until(end, ChronoUnit.NANOS) + (inclusiveEnd ? 1 : 0); } /** diff --git a/engine/time/src/test/java/io/deephaven/time/calendar/TestBusinessCalendarXMLParser.java b/engine/time/src/test/java/io/deephaven/time/calendar/TestBusinessCalendarXMLParser.java index a8a4a75ee0e..43635475aa7 100644 --- a/engine/time/src/test/java/io/deephaven/time/calendar/TestBusinessCalendarXMLParser.java +++ b/engine/time/src/test/java/io/deephaven/time/calendar/TestBusinessCalendarXMLParser.java @@ -36,6 +36,9 @@ public static void assertParserTestCal(final BusinessCalendar cal) { cal.calendarDay("2015-04-06").businessStart()); assertEquals(DateTimeUtils.parseInstant("2015-04-06T16:46 Asia/Tokyo"), cal.calendarDay("2015-04-06").businessEnd()); + + assertTrue(cal.calendarDay("2015-04-06").isInclusiveEnd()); + assertFalse(cal.calendarDay("2015-04-07").isInclusiveEnd()); } public void testLoad() throws URISyntaxException { diff --git a/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendarDay.java b/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendarDay.java index 367ebbdb7a0..93f2dd5520e 100644 --- a/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendarDay.java +++ b/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendarDay.java @@ -61,10 +61,10 @@ public void testSinglePeriod() { assertEquals(close1, single.businessEnd()); assertEquals(close1, single.businessEnd()); assertTrue(single.isInclusiveEnd()); - assertEquals(DateTimeUtils.HOUR, single.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR), single.businessDuration()); - assertEquals(DateTimeUtils.HOUR, single.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR), single.businessDuration()); + assertEquals(DateTimeUtils.HOUR + 1, single.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR + 1), single.businessDuration()); + assertEquals(DateTimeUtils.HOUR + 1, single.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR + 1), single.businessDuration()); assertTrue(single.isBusinessDay()); assertTrue(single.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:00:00.000000000 NY"))); assertTrue(single.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:15:00.000000000 NY"))); @@ -77,13 +77,13 @@ public void testSinglePeriod() { single.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); assertEquals(Duration.ofNanos(DateTimeUtils.MINUTE * 30), single.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR, + assertEquals(DateTimeUtils.HOUR + 1, single.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR + 1), single.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(DateTimeUtils.MINUTE * 30, + assertEquals(DateTimeUtils.MINUTE * 30 + 1, single.businessNanosRemaining(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.MINUTE * 30), + assertEquals(Duration.ofNanos(DateTimeUtils.MINUTE * 30 + 1), single.businessDurationRemaining(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); assertEquals(0L, single.businessNanosRemaining(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); @@ -105,10 +105,10 @@ public void testMultiPeriod() { assertEquals(close2, multi.businessEnd()); assertEquals(close2, multi.businessEnd()); assertTrue(multi.isInclusiveEnd()); - assertEquals(DateTimeUtils.HOUR * 6, multi.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6), multi.businessDuration()); - assertEquals(DateTimeUtils.HOUR * 6, multi.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6), multi.businessDuration()); + assertEquals(DateTimeUtils.HOUR * 6 + 2, multi.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6 + 2), multi.businessDuration()); + assertEquals(DateTimeUtils.HOUR * 6 + 2, multi.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6 + 2), multi.businessDuration()); assertTrue(multi.isBusinessDay()); assertTrue(multi.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:00:00.000000000 NY"))); assertTrue(multi.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:15:00.000000000 NY"))); @@ -122,25 +122,25 @@ public void testMultiPeriod() { multi.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); assertEquals(Duration.ofNanos(DateTimeUtils.MINUTE * 30), multi.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 2, + assertEquals(DateTimeUtils.HOUR * 2 + 1, multi.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2 + 1), multi.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 2, + assertEquals(DateTimeUtils.HOUR * 2 + 1, multi.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2 + 1), multi.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 6, + assertEquals(DateTimeUtils.HOUR * 6 + 2, multi.businessNanosRemaining(DateTimeUtils.parseInstant("2017-03-11T01:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6 + 2), multi.businessDurationRemaining(DateTimeUtils.parseInstant("2017-03-11T01:00:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 5 + DateTimeUtils.MINUTE * 30, + assertEquals(DateTimeUtils.HOUR * 5 + DateTimeUtils.MINUTE * 30 + 2, multi.businessNanosRemaining(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 5 + DateTimeUtils.MINUTE * 30), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 5 + DateTimeUtils.MINUTE * 30 + 2), multi.businessDurationRemaining(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 4, + assertEquals(DateTimeUtils.HOUR * 4 + 1, multi.businessNanosRemaining(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 4), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 4 + 1), multi.businessDurationRemaining(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); assertEquals(NULL_LONG, multi.businessNanosElapsed(null)); @@ -156,10 +156,10 @@ public void testMultiPeriod() { assertEquals(close2, multi2.businessEnd()); assertEquals(close2, multi2.businessEnd()); assertTrue(multi2.isInclusiveEnd()); - assertEquals(DateTimeUtils.HOUR * 6, multi2.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6), multi2.businessDuration()); - assertEquals(DateTimeUtils.HOUR * 6, multi2.businessNanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6), multi2.businessDuration()); + assertEquals(DateTimeUtils.HOUR * 6 + 2, multi2.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6 + 2), multi2.businessDuration()); + assertEquals(DateTimeUtils.HOUR * 6 + 2, multi2.businessNanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 6 + 2), multi2.businessDuration()); assertTrue(multi2.isBusinessDay()); assertTrue(multi2.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:00:00.000000000 NY"))); assertTrue(multi2.isBusinessTime(DateTimeUtils.parseInstant("2017-03-11T10:15:00.000000000 NY"))); @@ -173,13 +173,13 @@ public void testMultiPeriod() { multi2.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); assertEquals(Duration.ofNanos(DateTimeUtils.MINUTE * 30), multi2.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T10:30:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 2, + assertEquals(DateTimeUtils.HOUR * 2 + 1, multi2.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2 + 1), multi2.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(DateTimeUtils.HOUR * 2, + assertEquals(DateTimeUtils.HOUR * 2 + 1, multi2.businessNanosElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2), + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR * 2 + 1), multi2.businessDurationElapsed(DateTimeUtils.parseInstant("2017-03-11T13:00:00.000000000 NY"))); assertEquals(NULL_LONG, multi2.businessNanosElapsed(null)); diff --git a/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendars.java b/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendars.java index ef10b20e9d4..6c86dc632de 100644 --- a/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendars.java +++ b/engine/time/src/test/java/io/deephaven/time/calendar/TestCalendars.java @@ -5,6 +5,7 @@ import io.deephaven.base.testing.BaseArrayTestCase; import io.deephaven.configuration.Configuration; +import io.deephaven.time.DateTimeUtils; import java.net.URISyntaxException; import java.nio.file.Paths; @@ -82,6 +83,15 @@ public void testAdd() throws URISyntaxException { } catch (Exception e) { // pass } + } + + public void testUTCDayLength() { + final BusinessCalendar cal = Calendars.calendar("UTC"); + assertEquals(DateTimeUtils.DAY, cal.standardBusinessDay().businessNanos()); + } + public void testNYSEDayLength() { + final BusinessCalendar cal = Calendars.calendar("USNYSE_EXAMPLE"); + assertEquals(6 * DateTimeUtils.HOUR + 30 * DateTimeUtils.MINUTE, cal.standardBusinessDay().businessNanos()); } } diff --git a/engine/time/src/test/java/io/deephaven/time/calendar/TestTimeRange.java b/engine/time/src/test/java/io/deephaven/time/calendar/TestTimeRange.java index 5a0a11dbd87..476d7ee243f 100644 --- a/engine/time/src/test/java/io/deephaven/time/calendar/TestTimeRange.java +++ b/engine/time/src/test/java/io/deephaven/time/calendar/TestTimeRange.java @@ -50,8 +50,8 @@ public void testTimeRangeInclusive() { assertEquals(open1, period.start()); assertEquals(close1, period.end()); assertTrue(period.isInclusiveEnd()); - assertEquals(DateTimeUtils.HOUR, period.nanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR), period.duration()); + assertEquals(DateTimeUtils.HOUR + 1, period.nanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR + 1), period.duration()); assertTrue(period.contains(open1)); assertTrue(period @@ -101,8 +101,8 @@ public void testTimeRangeExclusive() { assertEquals(open1, period.start()); assertEquals(close1, period.end()); assertFalse(period.isInclusiveEnd()); - assertEquals(DateTimeUtils.HOUR - 1, period.nanos()); - assertEquals(Duration.ofNanos(DateTimeUtils.HOUR - 1), period.duration()); + assertEquals(DateTimeUtils.HOUR, period.nanos()); + assertEquals(Duration.ofNanos(DateTimeUtils.HOUR), period.duration()); assertTrue(period.contains(open1)); assertTrue(period @@ -192,4 +192,19 @@ public void testToStringExclusive() { final TimeRange p1 = new TimeRange<>(start, end, false); assertEquals("TimeRange{start=01:02:03, end=07:08:09, inclusiveEnd=false}", p1.toString()); } + + public void testNanos() { + final Instant t1 = DateTimeUtils.epochMillisToInstant(0); + final Instant t2 = DateTimeUtils.epochMillisToInstant(1); + + final TimeRange pInclusive = new TimeRange<>(t1, t2, true); + final TimeRange pExclusive = new TimeRange<>(t1, t2, false); + + final long nInclusive = pInclusive.nanos(); + final long nExclusive = pExclusive.nanos(); + + assertNotEquals(nInclusive, nExclusive); + assertEquals(DateTimeUtils.MILLI, nExclusive); + assertEquals(DateTimeUtils.MILLI + 1, nInclusive); + } } diff --git a/engine/time/src/test/resources/PARSER-TEST.calendar b/engine/time/src/test/resources/PARSER-TEST.calendar index 0fff2688674..3f4d43e1344 100644 --- a/engine/time/src/test/resources/PARSER-TEST.calendar +++ b/engine/time/src/test/resources/PARSER-TEST.calendar @@ -18,6 +18,6 @@ 2015-04-06 - 14:1516:46 + 14:1516:46true diff --git a/props/configs/src/main/resources/calendar/UTC.calendar b/props/configs/src/main/resources/calendar/UTC.calendar index 5b1f6c25fda..f3b1a8430d6 100644 --- a/props/configs/src/main/resources/calendar/UTC.calendar +++ b/props/configs/src/main/resources/calendar/UTC.calendar @@ -9,6 +9,6 @@ 1900-01-01 2100-12-31 - 00:0023:59:59.999999999 + 00:0023:59:59.999999999true \ No newline at end of file