Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improved handling for zero-length intervals. (#14136) 

* Improved handling for zero-length intervals.

1) Return an empty list from VersionedIntervalTimeline.lookup when
   provided with an empty interval. (The logic doesn't quite work when
   intervals are empty, which led to #14129.)

2) Don't return zero-length intervals from JodaUtils.condenseIntervals.

3) Detect "incorrect" comparator in JodaUtils.condenseIntervals, and
   recreate the SortedSet if needed. (Not strictly related to the theme
   of this patch. Just another thing in the same file.)

4) Remove unused method JodaUtils.containOverlappingIntervals.

Fixes #14129.

* Fix TimewarpOperatorTest.
This commit is contained in:
Gian Merlino 2023-04-25 17:12:56 -07:00 committed by GitHub
parent a7d4162195
commit 2dfb693d4c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 172 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
processing/src/main/java/org/apache/druid/java/util/common/JodaUtils.java
View File

@ -45,10 +45,11 @@ public class JodaUtils
public static final long MIN_INSTANT = Long.MIN_VALUE / 2;
/**
* This method will not materialize the input intervals if they represent
* a SortedSet (i.e. implement that interface). If not, the method internally
* creates a sorted set and populates it with them thus materializing the
* intervals in the input.
* Condense the provided intervals such that the returned list has no abutting intervals, no overlapping intervals,
* and no zero-length intervals.
* <p>
* This method is most efficient if the input intervals are in a {@link SortedSet} whose comparator is
* {@link Comparators#intervalsByStartThenEnd()}.
*
* @param intervals The Iterable object containing the intervals to condense
* @return The condensed intervals
@ -57,7 +58,8 @@ public class JodaUtils
{
final SortedSet<Interval> sortedIntervals;
if (intervals instanceof SortedSet) {
if (intervals instanceof SortedSet
&& (Comparators.intervalsByStartThenEnd().equals(((SortedSet<Interval>) intervals).comparator()))) {
sortedIntervals = (SortedSet<Interval>) intervals;
} else {
sortedIntervals = new TreeSet<>(Comparators.intervalsByStartThenEnd());
@ -69,30 +71,39 @@ public class JodaUtils
}
/**
* This method does not materialize the intervals represented by the
* sortedIntervals iterator. However, caller needs to insure that sortedIntervals
* is already sorted in ascending order (use the Comparators.intervalsByStartThenEnd()).
* It avoids materialization by incrementally condensing the intervals by
* Condense the provided intervals such that the returned list has no abutting intervals, no overlapping intervals,
* and no zero-length intervals.
* <p>
* The provided intervals must be sorted in ascending order using {@link Comparators#intervalsByStartThenEnd()}.
* Otherwise, results are undefined.
* <p>
* The method avoids materialization by incrementally condensing the intervals by
* starting from the first and looking for "adjacent" intervals. This is
* possible since intervals in the Iterator are in ascending order (as
* guaranteed by the caller).
* <p>
* *
*
* @param sortedIntervals The iterator object containing the intervals to condense
*
* @return An iterator for the condensed intervals. By construction the condensed intervals are sorted
* in ascending order and contain no repeated elements. The iterator can contain nulls,
* they will be skipped if it does.
*
* @throws IAE if an element is null or if sortedIntervals is not sorted in ascending order
*/
public static Iterator<Interval> condensedIntervalsIterator(Iterator<Interval> sortedIntervals)
{
if (sortedIntervals == null || !sortedIntervals.hasNext()) {
return Collections.emptyIterator();
}
final PeekingIterator<Interval> peekingIterator = Iterators.peekingIterator(sortedIntervals);
final PeekingIterator<Interval> peekingIterator =
Iterators.peekingIterator(
Iterators.filter(
sortedIntervals,
interval -> interval == null || interval.toDurationMillis() > 0
)
);
return new Iterator<Interval>()
{
private Interval previous;
@ -146,34 +157,6 @@ public class JodaUtils
};
}
/**
* Verify whether an iterable of intervals contains overlapping intervals
*
* @param intervals An interval iterable sorted using Comparators.intervalsByStartThenEnd()
* @return true if the iterable contains at least two overlapping intervals, false otherwise.
* @throws IAE when at least an element is null or when the iterable is not sorted
*/
public static boolean containOverlappingIntervals(Iterable<Interval> intervals)
{
if (intervals == null) {
return false;
}
boolean retVal = false;
Interval previous = null;
for (Interval current : intervals) {
if (current == null) {
throw new IAE("Intervals should not contain nulls");
}
verifyAscendingSortOrder(previous, current);
if (previous != null && previous.overlaps(current)) {
retVal = true;
break;
}
previous = current;
}
return retVal;
}
private static void verifyAscendingSortOrder(Interval previous, Interval current)
{
if (previous != null && previous.isAfter(current)) {

3
processing/src/main/java/org/apache/druid/query/spec/MultipleIntervalSegmentSpec.java
View File

@ -27,7 +27,6 @@ import org.apache.druid.query.QueryRunner;
import org.apache.druid.query.QuerySegmentWalker;
import org.joda.time.Interval;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
@ -42,7 +41,7 @@ public class MultipleIntervalSegmentSpec implements QuerySegmentSpec
@JsonProperty("intervals") List<Interval> intervals
)
{
this.intervals = Collections.unmodifiableList(JodaUtils.condenseIntervals(intervals));
this.intervals = JodaUtils.condenseIntervals(intervals);
}
@Override

5
processing/src/main/java/org/apache/druid/timeline/VersionedIntervalTimeline.java
View File

@ -35,6 +35,7 @@ import org.joda.time.Interval;
import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.IdentityHashMap;
@ -722,6 +723,10 @@ public class VersionedIntervalTimeline<VersionType, ObjectType extends Overshado
@GuardedBy("lock")
private List<TimelineObjectHolder<VersionType, ObjectType>> lookup(Interval interval, Partitions completeness)
{
if (interval.getStartMillis() == interval.getEndMillis()) {
return Collections.emptyList();
}
List<TimelineObjectHolder<VersionType, ObjectType>> retVal = new ArrayList<>();
NavigableMap<Interval, TimelineEntry> timeline;
if (completeness == Partitions.INCOMPLETE_OK) {

127
processing/src/test/java/org/apache/druid/common/utils/JodaUtilsTest.java
View File

@ -33,6 +33,8 @@ import org.junit.Test;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
/**
*
@ -269,6 +271,80 @@ public class JodaUtilsTest
}
}
@Test
public void testCondenseIntervalsUsingTreeSetWithIncorrectComparator()
{
final Set<Interval> intervals = new TreeSet<>(Comparators.intervalsByEndThenStart());
intervals.addAll(
Arrays.asList(
Intervals.of("2011-01-01/2011-01-02"),
Intervals.of("2011-01-02/2011-01-03"),
Intervals.of("2011-02-01/2011-02-05"),
Intervals.of("2011-02-01/2011-02-02"),
Intervals.of("2011-02-03/2011-02-08"),
Intervals.of("2011-03-01/2011-03-02"),
Intervals.of("2011-03-03/2011-03-04"),
Intervals.of("2011-03-05/2011-03-06"),
Intervals.of("2011-04-01/2011-04-05"),
Intervals.of("2011-04-02/2011-04-03"),
Intervals.of("2011-05-01/2011-05-05"),
Intervals.of("2011-05-02/2011-05-07")
)
);
Assert.assertEquals(
Arrays.asList(
Intervals.of("2011-01-01/2011-01-03"),
Intervals.of("2011-02-01/2011-02-08"),
Intervals.of("2011-03-01/2011-03-02"),
Intervals.of("2011-03-03/2011-03-04"),
Intervals.of("2011-03-05/2011-03-06"),
Intervals.of("2011-04-01/2011-04-05"),
Intervals.of("2011-05-01/2011-05-07")
),
JodaUtils.condenseIntervals(intervals)
);
}
@Test
public void testCondenseIntervalsMixedUpIncludingZeroLength()
{
List<Interval> intervals = Arrays.asList(
Intervals.of("2011-01-01/2011-01-02"),
Intervals.of("2011-01-02/2011-01-03"),
Intervals.of("2011-02-01/2011-02-01"),
Intervals.of("2011-02-01/2011-02-05"),
Intervals.of("2011-02-01/2011-02-02"),
Intervals.of("2011-02-03/2011-02-08"),
Intervals.of("2011-03-01/2011-03-02"),
Intervals.of("2011-03-03/2011-03-04"),
Intervals.of("2011-03-05/2011-03-06"),
Intervals.of("2011-04-01/2011-04-05"),
Intervals.of("2011-04-02/2011-04-03"),
Intervals.of("2011-05-01/2011-05-05"),
Intervals.of("2011-05-03/2011-05-03"),
Intervals.of("2011-05-02/2011-05-07"),
Intervals.of("2011-05-08/2011-05-08")
);
for (int i = 0; i < 20; ++i) {
Collections.shuffle(intervals);
Assert.assertEquals(
Arrays.asList(
Intervals.of("2011-01-01/2011-01-03"),
Intervals.of("2011-02-01/2011-02-08"),
Intervals.of("2011-03-01/2011-03-02"),
Intervals.of("2011-03-03/2011-03-04"),
Intervals.of("2011-03-05/2011-03-06"),
Intervals.of("2011-04-01/2011-04-05"),
Intervals.of("2011-05-01/2011-05-07")
),
JodaUtils.condenseIntervals(intervals)
);
}
}
@Test
public void testMinMaxInterval()
{
@ -290,55 +366,4 @@ public class JodaUtilsTest
final Period period = Intervals.ETERNITY.toDuration().toPeriod();
Assert.assertEquals(Long.MAX_VALUE, period.getMinutes());
}
@Test
public void testShouldContainOverlappingIntervals()
{
List<Interval> intervals = Arrays.asList(
Intervals.of("2011-02-01/2011-03-10"),
Intervals.of("2011-03-25/2011-04-03"),
Intervals.of("2011-04-01/2015-01-19"),
Intervals.of("2016-01-15/2016-01-19")
);
Assert.assertTrue(JodaUtils.containOverlappingIntervals(intervals));
}
@Test
public void testShouldNotContainOverlappingIntervals()
{
List<Interval> intervals = Arrays.asList(
Intervals.of("2011-02-01/2011-03-10"),
Intervals.of("2011-03-10/2011-04-03"),
Intervals.of("2011-04-04/2015-01-14"),
Intervals.of("2016-01-15/2016-01-19")
);
Assert.assertFalse(JodaUtils.containOverlappingIntervals(intervals));
}
@Test(expected = IAE.class)
public void testOverlappingIntervalsContainsNull()
{
List<Interval> intervals = Arrays.asList(
Intervals.of("2011-02-01/2011-03-10"),
null,
Intervals.of("2011-04-04/2015-01-14"),
Intervals.of("2016-01-15/2016-01-19")
);
JodaUtils.containOverlappingIntervals(intervals);
}
@Test(expected = IAE.class)
public void testOverlappingIntervalsContainsUnsorted()
{
List<Interval> intervals = Arrays.asList(
Intervals.of("2011-02-01/2011-03-10"),
Intervals.of("2011-03-10/2011-04-03"),
Intervals.of("2016-01-15/2016-01-19"),
Intervals.of("2011-04-04/2015-01-14")
);
JodaUtils.containOverlappingIntervals(intervals);
}
}

39
processing/src/test/java/org/apache/druid/query/TimewarpOperatorTest.java
View File

@ -309,28 +309,10 @@ public class TimewarpOperatorTest
public void testEmptyFutureInterval()
{
QueryRunner<Result<TimeseriesResultValue>> queryRunner = testOperator.postProcess(
new QueryRunner<Result<TimeseriesResultValue>>()
{
@Override
public Sequence<Result<TimeseriesResultValue>> run(
QueryPlus<Result<TimeseriesResultValue>> queryPlus,
ResponseContext responseContext
)
{
final Query<Result<TimeseriesResultValue>> query = queryPlus.getQuery();
return Sequences.simple(
ImmutableList.of(
new Result<>(
query.getIntervals().get(0).getStart(),
new TimeseriesResultValue(ImmutableMap.of("metric", 2))
),
new Result<>(
query.getIntervals().get(0).getEnd(),
new TimeseriesResultValue(ImmutableMap.of("metric", 3))
)
)
);
}
(queryPlus, responseContext) -> {
final Query<Result<TimeseriesResultValue>> query = queryPlus.getQuery();
Assert.assertTrue(query.getIntervals().isEmpty());
return Sequences.empty();
},
DateTimes.of("2014-08-02").getMillis()
);
@ -338,21 +320,12 @@ public class TimewarpOperatorTest
final Query<Result<TimeseriesResultValue>> query =
Druids.newTimeseriesQueryBuilder()
.dataSource("dummy")
.intervals("2014-08-06/2014-08-08")
.intervals("2014-08-06/2014-08-08") // Warped interval is zero-length
.aggregators(Collections.singletonList(new CountAggregatorFactory("count")))
.build();
Assert.assertEquals(
Lists.newArrayList(
new Result<>(
DateTimes.of("2014-08-02"),
new TimeseriesResultValue(ImmutableMap.of("metric", 2))
),
new Result<>(
DateTimes.of("2014-08-02"),
new TimeseriesResultValue(ImmutableMap.of("metric", 3))
)
),
Collections.emptyList(),
queryRunner.run(QueryPlus.wrap(query)).toList()
);
}

60
processing/src/test/java/org/apache/druid/timeline/VersionedIntervalTimelineTest.java
View File

@ -470,6 +470,66 @@ public class VersionedIntervalTimelineTest extends VersionedIntervalTimelineTest
);
}
// 1|----|----|
// 2|---------|
@Test
public void testOverlapSecondContainsFirstZeroLengthInterval()
{
add("2011-01-01/2011-01-20", "1", 1);
add("2011-01-01/2011-01-10", "2", 2);
add("2011-01-10/2011-01-20", "2", 3);
assertValues(
Collections.emptyList(),
timeline.lookup(Intervals.of("2011-01-10/2011-01-10"))
);
}
// 1|----|----|
// 2|---------|
@Test
public void testOverlapSecondContainsFirstZeroLengthInterval2()
{
add("2011-01-01/2011-01-20", "1", 1);
add("2011-01-01/2011-01-10", "2", 2);
add("2011-01-10/2011-01-20", "2", 3);
assertValues(
Collections.emptyList(),
timeline.lookup(Intervals.of("2011-01-11/2011-01-11"))
);
}
// 1|----|----|
// 2|---------|
@Test
public void testOverlapSecondContainsFirstZeroLengthInterval3()
{
add("2011-01-01/2011-01-20", "1", 1);
add("2011-01-01/2011-01-10", "2", 2);
add("2011-01-10/2011-01-20", "2", 3);
assertValues(
Collections.emptyList(),
timeline.lookup(Intervals.of("2011-01-01/2011-01-01"))
);
}
// 1|----|----|
// 2|---------|
@Test
public void testOverlapSecondContainsFirstZeroLengthInterval4()
{
add("2011-01-01/2011-01-20", "1", 1);
add("2011-01-01/2011-01-10", "2", 2);
add("2011-01-10/2011-01-20", "2", 3);
assertValues(
Collections.emptyList(),
timeline.lookup(Intervals.of("2011-01-20/2011-01-20"))
);
}
// 2|---------|
// 1|----|----|
@Test