AmatyaAvadhanula commented on code in PR #12477:
URL: https://github.com/apache/druid/pull/12477#discussion_r856884272


##########
core/src/main/java/org/apache/druid/timeline/partition/DimensionRangeShardSpec.java:
##########
@@ -223,16 +208,29 @@ public boolean possibleInDomain(Map<String, 
RangeSet<String>> domain)
 
       // EffectiveDomain[i] = QueryDomain[i] INTERSECTION SegmentRange[i]
       RangeSet<String> effectiveDomainForDimension = 
queryDomainForDimension.subRangeSet(rangeTillSegmentBoundary);
+
+      // Create an iterator to use for checking if the RangeSet is empty or is 
a singleton. This is significantly
+      // faster than using isEmpty() and equals(), because those methods call 
size() internally, which iterates
+      // the entire RangeSet.
+      final Iterator<Range<String>> effectiveDomainRangeIterator = 
effectiveDomainForDimension.asRanges().iterator();
+
       // Prune segment because query domain is out of segment range
-      if (effectiveDomainForDimension.isEmpty()) {
+      if (!effectiveDomainRangeIterator.hasNext()) {
         return false;
       }
 
-      // EffectiveDomain is singleton and lies only on the boundaries -> 
consider next dimensions
-      effectiveDomainIsStart = effectiveDomainIsStart
-                                && 
isRangeSetSingletonWithVal(effectiveDomainForDimension, segmentStart.get(i));
-      effectiveDomainIsEnd = effectiveDomainIsEnd
-                           && 
isRangeSetSingletonWithVal(effectiveDomainForDimension, segmentEnd.get(i));
+      final Range<String> firstRange = effectiveDomainRangeIterator.next();
+
+      if (!effectiveDomainRangeIterator.hasNext()) {
+        // Effective domain contained only one Range.
+        // If it's a singleton and lies only on the boundaries -> consider 
next dimensions
+        effectiveDomainIsStart = effectiveDomainIsStart
+                                 && segmentStart.get(i) != null
+                                 && 
firstRange.equals(Range.singleton(segmentStart.get(i)));
+        effectiveDomainIsEnd = effectiveDomainIsEnd
+                               && segmentEnd.get(i) != null
+                               && 
firstRange.equals(Range.singleton(segmentEnd.get(i)));
+      }

Review Comment:
   `else {
     return true;
   }`
   This snippet might be required at this point since effective domains are no 
longer singleton, and we cannot prune further?
   If not, we may go to the next dimension and prune a segment that must not be 
pruned.



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