Interval optimization for subclasses of DataSegment (apache#18594)

* Interval optimization for subclasses of DataSegment

* Rename parameter name to serializedInterval

Author: GWphua

multi-stage-query/src/main/java/org/apache/druid/msq/input/table/DataSegmentWithLocation.java

@@ -25,11 +25,11 @@
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.common.base.Preconditions;
 import org.apache.druid.jackson.CommaListJoinDeserializer;
+import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.server.coordination.DruidServerMetadata;
 import org.apache.druid.timeline.CompactionState;
 import org.apache.druid.timeline.DataSegment;
 import org.apache.druid.timeline.partition.ShardSpec;
-import org.joda.time.Interval;
 
 import javax.annotation.Nullable;
 import java.util.List;
@@ -45,9 +45,9 @@ public class DataSegmentWithLocation extends DataSegment
   private final Set<DruidServerMetadata> servers;
 
   @JsonCreator
-  public DataSegmentWithLocation(
+  private DataSegmentWithLocation(
       @JsonProperty("dataSource") String dataSource,
-      @JsonProperty("interval") Interval interval,
+      @JsonProperty("interval") String interval,
       @JsonProperty("version") String version,
       // use `Map` *NOT* `LoadSpec` because we want to do lazy materialization to prevent dependency pollution
       @JsonProperty("loadSpec") @Nullable Map<String, Object> loadSpec,
@@ -66,7 +66,7 @@ public DataSegmentWithLocation(
   {
     super(
         dataSource,
-        interval,
+        Intervals.fromString(interval),
         version,
         loadSpec,
         dimensions,

processing/src/main/java/org/apache/druid/java/util/common/Intervals.java

@@ -78,36 +78,39 @@ public static Interval of(String format, Object... formatArgs)
    *
    * Currently, this method is only used in {@link org.apache.druid.timeline.SegmentId}.
    */
-  public static Interval fromString(String string)
+  public static Interval fromString(String serializedInterval)
   {
-    Interval interval = null;
-    if (canDeserializeIntervalOptimallyFromString(string)) {
-      interval = tryOptimizedIntervalDeserialization(string);
+    if (canDeserializeIntervalOptimallyFromString(serializedInterval)) {
+      Interval interval = tryOptimizedIntervalDeserialization(serializedInterval);
+
+      if (interval != null) {
+        return interval;
+      }
     }
 
-    return interval == null ? Intervals.of(string) : interval;
+    return Intervals.of(serializedInterval);
   }
 
-  private static boolean canDeserializeIntervalOptimallyFromString(String intervalText)
+  private static boolean canDeserializeIntervalOptimallyFromString(String serializedInterval)
   {
     // Optimized version does not deal well with Periods.
-    if (intervalText.contains("P")) {
+    if (serializedInterval.contains("P")) {
       return false;
     }
 
-    final int slashIndex = intervalText.indexOf('/');
-    return (slashIndex > 0 && slashIndex < intervalText.length() - 1);
+    final int slashIndex = serializedInterval.indexOf('/');
+    return (slashIndex > 0 && slashIndex < serializedInterval.length() - 1);
   }
 
   /**
    * @return null if the input format cannot be parsed with optimized strategy, else return the Interval.
    */
   @Nullable
-  private static Interval tryOptimizedIntervalDeserialization(final String intervalText)
+  private static Interval tryOptimizedIntervalDeserialization(final String serializedInterval)
   {
-    final int slashIndex = intervalText.indexOf('/');
-    final String startStr = intervalText.substring(0, slashIndex);
-    final String endStr = intervalText.substring(slashIndex + 1);
+    final int slashIndex = serializedInterval.indexOf('/');
+    final String startStr = serializedInterval.substring(0, slashIndex);
+    final String endStr = serializedInterval.substring(slashIndex + 1);
 
     try {
       final long startMillis = FAST_ISO_UTC_FORMATTER.parseMillis(startStr);

server/src/main/java/org/apache/druid/server/coordination/LoadableDataSegment.java

@@ -23,10 +23,10 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import org.apache.druid.jackson.CommaListJoinDeserializer;
+import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.timeline.CompactionState;
 import org.apache.druid.timeline.DataSegment;
 import org.apache.druid.timeline.partition.ShardSpec;
-import org.joda.time.Interval;
 
 import javax.annotation.Nullable;
 import java.util.List;
@@ -35,15 +35,15 @@
 /**
  * A deserialization aid used by {@link SegmentChangeRequestLoad}. The broker prunes the loadSpec from segments
  * for efficiency reasons, but the broker does need the loadSpec when it loads broadcast segments.
- *
+ * <p>
  * This class always uses the non-pruning default {@link PruneSpecsHolder}.
  */
 public class LoadableDataSegment extends DataSegment
 {
   @JsonCreator
-  public LoadableDataSegment(
+  private LoadableDataSegment(
       @JsonProperty("dataSource") String dataSource,
-      @JsonProperty("interval") Interval interval,
+      @JsonProperty("interval") String interval,
       @JsonProperty("version") String version,
       // use `Map` *NOT* `LoadSpec` because we want to do lazy materialization to prevent dependency pollution
       @JsonProperty("loadSpec") @Nullable Map<String, Object> loadSpec,
@@ -58,7 +58,7 @@ public LoadableDataSegment(
   {
     super(
         dataSource,
-        interval,
+        Intervals.fromString(interval),
         version,
         loadSpec,
         dimensions,
@@ -70,6 +70,5 @@ public LoadableDataSegment(
         size,
         PruneSpecsHolder.DEFAULT
     );
-
   }
 }