GWphua commented on code in PR #18477:
URL: https://github.com/apache/druid/pull/18477#discussion_r2321525496
##########
processing/src/test/java/org/apache/druid/java/util/common/IntervalsTest.java:
##########
@@ -79,6 +79,44 @@ public void testFindOverlappingInterval()
);
}
+ @Test
+ public void testValidIntervalStrings()
+ {
+ final String[] intervalStringRepresentations = new String[]{
+ // Tests that use does not fallback to Intervals.of()
+ // Zulu with millis
+ "2022-01-01T00:00:00.000Z/2022-01-02T00:00:00.000Z",
+ "2021-03-14T12:34:56.789Z/2021-03-15T12:34:56.789Z",
+
+ // Offset with colon
+ "2022-01-01T00:00:00.000+05:30/2022-01-01T01:00:00.000+05:30",
+ "2022-01-01T07:00:00.000-07:00/2022-01-01T08:00:00.000-07:00",
+
+ // Basic offset without colon
+ "2022-01-01T00:00:00.000+0530/2022-01-01T01:00:00.000+0530",
+
+ // Tests that fallback to Intervals.of()
+ // Zulu without millis
+ "2022-01-01T00:00:00Z/2022-01-02T00:00:00Z",
+ // Date-only
+ "2022-01-01/2022-01-02",
+ // start/period
+ "2022-01-01T00:00:00.000Z/P1D",
+ "2022-01-01T12:00:00Z/PT6H",
+ "2022-01-01T00:00:00Z/P2DT3H4M5S",
+ // period/end
+ "P1D/2022-01-02T00:00:00.000Z",
+ "PT6H/2022-01-01T18:00:00Z",
+ "P2DT3H4M5S/2022-01-03T03:04:05Z"
Review Comment:
> if we could quickly identify that the string has a period/end
configuration, we could parse both parts separately.
Regarding this, I have this solution in mind:
1. Suppose start is a timestamp, end is a period string.
2. Parse the period string to a long.
3. `endMillis` = period in long + `startMillis`
This method does not work all the time though. We may not get the correct
number for leap years, or even months. We will need to calculate start/end
period with awareness of these semantics, which is more complex.
I feel we can stick with knowing that we are dealing with the most common
data format (`"YYYY-MM-DD'T'HH:mm:ss.SSSZ/YYYY-MM-DD'T'HH:mm:ss.SSSZ"`) for
this method, and use `Interval.of(String)` for other formats?
##########
processing/src/test/java/org/apache/druid/java/util/common/IntervalsTest.java:
##########
@@ -79,6 +79,44 @@ public void testFindOverlappingInterval()
);
}
+ @Test
+ public void testValidIntervalStrings()
+ {
+ final String[] intervalStringRepresentations = new String[]{
+ // Tests that use does not fallback to Intervals.of()
+ // Zulu with millis
+ "2022-01-01T00:00:00.000Z/2022-01-02T00:00:00.000Z",
+ "2021-03-14T12:34:56.789Z/2021-03-15T12:34:56.789Z",
+
+ // Offset with colon
+ "2022-01-01T00:00:00.000+05:30/2022-01-01T01:00:00.000+05:30",
+ "2022-01-01T07:00:00.000-07:00/2022-01-01T08:00:00.000-07:00",
+
+ // Basic offset without colon
+ "2022-01-01T00:00:00.000+0530/2022-01-01T01:00:00.000+0530",
+
+ // Tests that fallback to Intervals.of()
+ // Zulu without millis
+ "2022-01-01T00:00:00Z/2022-01-02T00:00:00Z",
+ // Date-only
+ "2022-01-01/2022-01-02",
+ // start/period
+ "2022-01-01T00:00:00.000Z/P1D",
+ "2022-01-01T12:00:00Z/PT6H",
+ "2022-01-01T00:00:00Z/P2DT3H4M5S",
+ // period/end
+ "P1D/2022-01-02T00:00:00.000Z",
+ "PT6H/2022-01-01T18:00:00Z",
+ "P2DT3H4M5S/2022-01-03T03:04:05Z"
Review Comment:
> if we could quickly identify that the string has a period/end
configuration, we could parse both parts separately.
Regarding this, I have this solution in mind:
1. Suppose start is a timestamp, end is a period string.
2. Parse the period string to a long.
3. `endMillis` = period in long + `startMillis`
This method does not work all the time though. We may not get the correct
number for leap years, or even months. We will need to calculate start/end
period with awareness of these semantics, which is more complex.
I feel we can stick with knowing that we are dealing with the most common
data format (`"YYYY-MM-DD'T'HH:mm:ss.SSSZ/YYYY-MM-DD'T'HH:mm:ss.SSSZ"`) for
this method, and use `Interval.of(String)` for other formats?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
