From aa62073faad2d64474a6065f20c4ce1573263bc7 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Sat, 15 May 2021 14:26:41 -0700 Subject: [PATCH] fix sql planner bug with inner offset causing loop (#11259) * fix sql planner bug with inner offset causing loop * move check up --- .../druid/sql/calcite/rel/DruidQuery.java | 10 +-- .../druid/sql/calcite/CalciteQueryTest.java | 61 +++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java index 251340fc3e3..2a10cd409a5 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java @@ -780,6 +780,11 @@ public class DruidQuery return null; } + if (sorting != null && sorting.getOffsetLimit().hasOffset()) { + // Timeseries cannot handle offsets. + return null; + } + final Granularity queryGranularity; final boolean descending; int timeseriesLimit = 0; @@ -803,11 +808,6 @@ public class DruidQuery Iterables.getOnlyElement(grouping.getDimensions()).toDimensionSpec().getOutputName() ); if (sorting != null) { - if (sorting.getOffsetLimit().hasOffset()) { - // Timeseries cannot handle offsets. - return null; - } - if (sorting.getOffsetLimit().hasLimit()) { final long limit = sorting.getOffsetLimit().getLimit(); 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 9a3ad28c763..b06967ac5c6 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 @@ -17394,4 +17394,65 @@ public class CalciteQueryTest extends BaseCalciteQueryTest ) ); } + + @Test + public void testEmptyGroupWithOffsetDoesntInfiniteLoop() throws Exception + { + testQuery( + "SELECT r0.c, r1.c\n" + + "FROM (\n" + + " SELECT COUNT(*) AS c\n" + + " FROM \"foo\"\n" + + " GROUP BY ()\n" + + " OFFSET 1\n" + + ") AS r0\n" + + "LEFT JOIN (\n" + + " SELECT COUNT(*) AS c\n" + + " FROM \"foo\"\n" + + " GROUP BY ()\n" + + ") AS r1 ON TRUE LIMIT 10", + ImmutableList.of( + Druids.newScanQueryBuilder() + .dataSource( + join( + new QueryDataSource( + GroupByQuery.builder() + .setDataSource(CalciteTests.DATASOURCE1) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setAggregatorSpecs( + aggregators( + new CountAggregatorFactory("a0") + ) + ) + .setLimitSpec(DefaultLimitSpec.builder().offset(1).limit(10).build()) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + new QueryDataSource( + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .granularity(Granularities.ALL) + .aggregators(new CountAggregatorFactory("a0")) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + "j0.", + "1", + JoinType.LEFT, + null + ) + ) + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("a0", "j0.a0") + .limit(10) + .resultFormat(ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .context(QUERY_CONTEXT_DEFAULT) + .legacy(false) + .build() + ), + ImmutableList.of() + ); + } }