You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by ab...@apache.org on 2022/09/07 11:15:21 UTC

[druid] branch master updated: Disallow timeseries queries with ETERNITY interval and non-ALL granularity (#12944)

This is an automated email from the ASF dual-hosted git repository.

abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new 2f156b3610 Disallow timeseries queries with ETERNITY interval and non-ALL granularity (#12944)
2f156b3610 is described below

commit 2f156b3610d47983819f928e774ccbe3f067e1e4
Author: Rohan Garg <77...@users.noreply.github.com>
AuthorDate: Wed Sep 7 16:45:08 2022 +0530

    Disallow timeseries queries with ETERNITY interval and non-ALL granularity (#12944)
---
 .../druid/segment/RowBasedStorageAdapter.java      |  8 ++++++
 .../druid/segment/RowBasedStorageAdapterTest.java  | 17 ++++++++++++
 .../apache/druid/sql/calcite/CalciteQueryTest.java | 32 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java
index 29d68aeb41..3fed0fc1cd 100644
--- a/processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java
+++ b/processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java
@@ -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,
diff --git a/processing/src/test/java/org/apache/druid/segment/RowBasedStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/RowBasedStorageAdapterTest.java
index f2c5502b69..b13953286d 100644
--- a/processing/src/test/java/org/apache/druid/segment/RowBasedStorageAdapterTest.java
+++ b/processing/src/test/java/org/apache/druid/segment/RowBasedStorageAdapterTest.java
@@ -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
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 15be7d30b2..7115e3ca0c 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -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()
+        )
+    );
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org