Skip to content

Commit

Permalink
fix!: Fix a TimeRange off-by-one bug in nanosecond calculation (deeph…
Browse files Browse the repository at this point in the history
…aven#5648)

Fix a TimeRange off-by-one bug in nanosecond calculation.

Resolves deephaven#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
`<includeClose>true</includeClose>` 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
`<includeClose>true</includeClose>`.
  • Loading branch information
chipkent committed Aug 28, 2024
1 parent 737d17c commit e75b192
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@
* <description>New York Stock Exchange Calendar</description>
* <timeZone>America/New_York</timeZone>
* <default>
* <businessTime><open>09:30</open><close>16:00</close></businessTime>
* <businessTime>
* <open>09:30</open>
* <close>16:00</close>
* <!-- Optional include the close nanosecond in the business time range. -->
* <includeClose>true</includeClose>
* </businessTime>
* <weekend>Saturday</weekend>
* <weekend>Sunday</weekend>
* </default>
Expand All @@ -51,6 +56,8 @@
* <businessTime>
* <open>09:30</open>
* <close>13:00</close>
* <!-- Optional include the close nanosecond in the business time range. -->
* <includeClose>true</includeClose>
* </businessTime>
* </holiday>
* </calendar>
Expand Down Expand Up @@ -254,6 +261,7 @@ private static TimeRange<LocalTime>[] parseBusinessRanges(final List<Element> 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
Expand All @@ -262,7 +270,8 @@ private static TimeRange<LocalTime>[] parseBusinessRanges(final List<Element> 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;
Expand All @@ -282,7 +291,7 @@ private static TimeRange<LocalTime>[] parseBusinessRangesLegacy(final List<Eleme
final String closeTxt = openClose[1];
final LocalTime open = DateTimeUtils.parseLocalTime(openTxt);
final LocalTime close = DateTimeUtils.parseLocalTime(closeTxt);
rst[i] = new TimeRange<>(open, close, true);
rst[i] = new TimeRange<>(open, close, false);
} else {
throw new IllegalArgumentException("Can not parse business periods; open/close = " + br.getText());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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));
Expand All @@ -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")));
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -192,4 +192,19 @@ public void testToStringExclusive() {
final TimeRange<LocalTime> 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<Instant> pInclusive = new TimeRange<>(t1, t2, true);
final TimeRange<Instant> 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);
}
}
2 changes: 1 addition & 1 deletion engine/time/src/test/resources/PARSER-TEST.calendar
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
</holiday>
<holiday>
<date>2015-04-06</date>
<businessTime><open>14:15</open><close>16:46</close></businessTime>
<businessTime><open>14:15</open><close>16:46</close><includeClose>true</includeClose></businessTime>
</holiday>
</calendar>
2 changes: 1 addition & 1 deletion props/configs/src/main/resources/calendar/UTC.calendar
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
<firstValidDate>1900-01-01</firstValidDate>
<lastValidDate>2100-12-31</lastValidDate>
<default>
<businessTime><open>00:00</open><close>23:59:59.999999999</close></businessTime>
<businessTime><open>00:00</open><close>23:59:59.999999999</close><includeClose>true</includeClose></businessTime>
</default>
</calendar>

0 comments on commit e75b192

Please sign in to comment.