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?



-- 
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]

Reply via email to