Disallow timeseries queries with ETERNITY interval and non-ALL granularity (#12944)

This commit is contained in:
Rohan Garg 2022-09-07 16:45:08 +05:30 committed by GitHub
parent 7aa8d7f987
commit 2f156b3610
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 0 deletions

View File

@ -22,7 +22,9 @@ package org.apache.druid.segment;
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.common.granularity.Granularity;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.guava.Sequences;
@ -170,6 +172,12 @@ public class RowBasedStorageAdapter<RowType> implements StorageAdapter
if (actualInterval == null) {
return Sequences.empty();
}
// Incase time interval is ETERNITY with non-ALL granularity, don't risk creating time grains.
// For all non-ALL granularities, the time grains will be very high in number and that can either OOM the heap
// or create an very very long query.
if (actualInterval.contains(Intervals.ETERNITY) && !gran.equals(Granularities.ALL)) {
throw new IAE("Cannot support ETERNITY interval with %s time granluarity", gran);
}
final RowWalker<RowType> rowWalker = new RowWalker<>(
descending ? reverse(rowSequence) : rowSequence,

View File

@ -24,6 +24,7 @@ import com.google.common.collect.Lists;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.common.guava.GuavaUtils;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.granularity.Granularities;
@ -824,6 +825,22 @@ public class RowBasedStorageAdapterTest
Assert.assertEquals(1, numCloses.get());
}
@Test
public void test_makeCursors_eternityIntervalWithMonthGranularity()
{
final RowBasedStorageAdapter<Integer> adapter = createIntAdapter(0, 1);
Assert.assertThrows(IAE.class, () -> {
adapter.makeCursors(
null,
Intervals.ETERNITY,
VirtualColumns.EMPTY,
Granularities.MONTH,
false,
null
);
});
}
private static List<List<Object>> walkCursors(
final Sequence<Cursor> cursors,
final List<Function<Cursor, Supplier<Object>>> processors

View File

@ -28,6 +28,7 @@ import org.apache.calcite.runtime.CalciteContextException;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.HumanReadableBytes;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.JodaUtils;
import org.apache.druid.java.util.common.StringUtils;
@ -14120,4 +14121,35 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
)
);
}
@Test
public void testTimeseriesQueryWithEmptyInlineDatasourceAndGranularity()
{
// the SQL query contains an always FALSE filter ('bar' = 'baz'), which optimizes the query to also remove time
// filter. the converted query hence contains ETERNITY interval but still a MONTH granularity due to the grouping.
// Such a query should fail since it will create a huge amount of time grains which can lead to OOM or a very very
// high query time.
Assert.assertThrows(IAE.class, () ->
testQuery(
"SELECT TIME_FLOOR(__time, 'P1m'), max(m1) from \"foo\"\n"
+ "WHERE __time > CURRENT_TIMESTAMP - INTERVAL '3' MONTH AND 'bar'='baz'\n"
+ "GROUP BY 1\n"
+ "ORDER BY 1 DESC",
ImmutableList.of(
Druids.newTimeseriesQueryBuilder()
.dataSource(
InlineDataSource.fromIterable(
ImmutableList.of(),
RowSignature.builder().addTimeColumn().add("m1", ColumnType.STRING).build()
))
.intervals(ImmutableList.of(Intervals.ETERNITY))
.descending(true)
.granularity(Granularities.MONTH)
.aggregators(new LongMaxAggregatorFactory("a0", "m1"))
.build()
),
ImmutableList.of()
)
);
}
}