AmatyaAvadhanula commented on code in PR #12477:
URL: https://github.com/apache/druid/pull/12477#discussion_r856891081
##########
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:
@Test
public void testPossibleInDomain_FalsePruning()
{
setDimensions("planet", "country", "city");
final StringTuple start = StringTuple.create("Earth", "France",
"Paris");
final StringTuple end = StringTuple.create("Mars", "USA", "New York");
final RangeSet<String> universalSet = TreeRangeSet.create();
universalSet.add(Range.all());
ShardSpec shard = new DimensionRangeShardSpec(dimensions, start, end,
0, null);
Map<String, RangeSet<String>> domain = new HashMap<>();
// {Earth} U (Mars, INF) * (USA, INF) * (-INF, INF)
populateDomain(
domain,
getUnion(
getRangeSet(Range.singleton("Earth")),
getRangeSet(Range.singleton("Mars"))
),
getUnion(
getRangeSet(Range.greaterThan("USA"))
),
universalSet
);
assertTrue(shard.possibleInDomain(domain));
}
--
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]
