You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by vo...@apache.org on 2023/04/21 22:41:35 UTC
[druid] branch 26.0.0 updated: SQL planning: Consider subqueries in fewer scenarios. (#14123) (#14140)
This is an automated email from the ASF dual-hosted git repository.
vogievetsky pushed a commit to branch 26.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/26.0.0 by this push:
new 37983b83a4 SQL planning: Consider subqueries in fewer scenarios. (#14123) (#14140)
37983b83a4 is described below
commit 37983b83a47a62dfb29000036d9444971686633e
Author: Gian Merlino <gi...@gmail.com>
AuthorDate: Fri Apr 21 15:41:28 2023 -0700
SQL planning: Consider subqueries in fewer scenarios. (#14123) (#14140)
* SQL planning: Consider subqueries in fewer scenarios.
Further adjusts logic in DruidRules that was previously adjusted in #13902.
The reason for the original change was that the comment "Subquery must be
a groupBy, so stage must be >= AGGREGATE" was no longer accurate. Subqueries
do not need to be groupBy anymore; they can really be any type of query.
If I recall correctly, the change was needed for certain window queries
to be able to plan on top of Scan queries.
However, this impacts performance negatively, because it causes many
additional outer-query scenarios to be considered, which is expensive.
So, this patch updates the matching logic to consider fewer scenarios. The
skipped scenarios are ones where we expect that, for one reason or another,
it isn't necessary to consider a subquery.
* Remove unnecessary escaping.
* Fix test.
---
.../druid/sql/calcite/rel/PartialDruidQuery.java | 9 +++-
.../apache/druid/sql/calcite/rule/DruidRules.java | 28 ++++++++++--
.../druid/sql/calcite/CalciteJoinQueryTest.java | 10 +++--
.../apache/druid/sql/calcite/CalciteQueryTest.java | 51 +++++++++++-----------
4 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java
index 727e448b17..068ff49308 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java
@@ -78,7 +78,14 @@ public class PartialDruidQuery
// WHERE_FILTER, SELECT_PROJECT may be present on any query, except ones with WINDOW.
WHERE_FILTER,
- SELECT_PROJECT,
+ SELECT_PROJECT {
+ @Override
+ public boolean canFollow(Stage stage)
+ {
+ // SELECT_PROJECT can be stacked on top of another SELECT_PROJECT.
+ return stage.compareTo(this) <= 0;
+ }
+ },
// AGGREGATE, HAVING_FILTER, AGGREGATE_PROJECT can be present on non-WINDOW aggregating queries.
AGGREGATE,
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java
index 52841fd99a..b5e91916cf 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java
@@ -313,9 +313,31 @@ public class DruidRules
@Override
public boolean matches(final RelOptRuleCall call)
{
- // Only consider doing a subquery when the stage cannot be fused into a single query.
- final DruidRel<?> druidRel = call.rel(call.getRelList().size() - 1);
- return !stage.canFollow(druidRel.getPartialDruidQuery().stage());
+ final DruidRel<?> lowerDruidRel = call.rel(call.getRelList().size() - 1);
+ final RelNode lowerRel = lowerDruidRel.getPartialDruidQuery().leafRel();
+ final PartialDruidQuery.Stage lowerStage = lowerDruidRel.getPartialDruidQuery().stage();
+
+ if (stage.canFollow(lowerStage)
+ || (stage == PartialDruidQuery.Stage.WHERE_FILTER
+ && PartialDruidQuery.Stage.HAVING_FILTER.canFollow(lowerStage))
+ || (stage == PartialDruidQuery.Stage.SELECT_PROJECT
+ && PartialDruidQuery.Stage.SORT_PROJECT.canFollow(lowerStage))) {
+ // Don't consider cases that can be fused into a single query.
+ return false;
+ } else if (stage == PartialDruidQuery.Stage.WHERE_FILTER && lowerRel instanceof Filter) {
+ // Don't consider filter-on-filter. FilterMergeRule will handle it.
+ return false;
+ } else if (stage == PartialDruidQuery.Stage.WHERE_FILTER
+ && lowerStage == PartialDruidQuery.Stage.SELECT_PROJECT) {
+ // Don't consider filter-on-project. ProjectFilterTransposeRule will handle it by swapping them.
+ return false;
+ } else if (stage == PartialDruidQuery.Stage.SELECT_PROJECT && lowerRel instanceof Project) {
+ // Don't consider project-on-project. ProjectMergeRule will handle it.
+ return false;
+ } else {
+ // Consider subqueries in all other cases.
+ return true;
+ }
}
}
}
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
index 9b53d150bf..015b98d6d0 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
@@ -2718,8 +2718,9 @@ public class CalciteJoinQueryTest extends BaseCalciteQueryTest
JoinType.LEFT
)
)
+ .virtualColumns(expressionVirtualColumn("_v0", "'10.1'", ColumnType.STRING))
.intervals(querySegmentSpec(Filtration.eternity()))
- .columns("__time", "v0")
+ .columns("__time", "_v0")
.filters(new SelectorDimFilter("v0", "10.1", null))
.context(queryContext)
.build()
@@ -2829,8 +2830,10 @@ public class CalciteJoinQueryTest extends BaseCalciteQueryTest
JoinType.LEFT
)
)
+ .virtualColumns(expressionVirtualColumn("_v0", "'10.1'", ColumnType.STRING))
.intervals(querySegmentSpec(Filtration.eternity()))
- .columns("__time", "v0")
+ .filters(selector("v0", "10.1", null))
+ .columns("__time", "_v0")
.context(queryContext)
.build()
),
@@ -3022,8 +3025,9 @@ public class CalciteJoinQueryTest extends BaseCalciteQueryTest
JoinType.INNER
)
)
+ .virtualColumns(expressionVirtualColumn("_v0", "'10.1'", ColumnType.STRING))
.intervals(querySegmentSpec(Filtration.eternity()))
- .columns("__time", "v0")
+ .columns("__time", "_v0")
.context(queryContext);
testQuery(
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 9d5b0e2c52..dd8f8187f5 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
@@ -6898,12 +6898,9 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
.setInterval(querySegmentSpec(Filtration.eternity()))
.setGranularity(Granularities.ALL)
.setDimensions(
- useDefault ? dimensions(
+ dimensions(
new DefaultDimensionSpec("m2", "d0", ColumnType.DOUBLE),
new DefaultDimensionSpec("dim1", "d1")
- ) : dimensions(
- new DefaultDimensionSpec("dim1", "d0"),
- new DefaultDimensionSpec("m2", "d1", ColumnType.DOUBLE)
)
)
.setDimFilter(new SelectorDimFilter("m1", "5.0", null))
@@ -6922,7 +6919,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
)
.setDimensions(dimensions(
new DefaultDimensionSpec("v0", "_d0", ColumnType.LONG),
- new DefaultDimensionSpec(useDefault ? "d1" : "d0", "_d1", ColumnType.STRING)
+ new DefaultDimensionSpec("d1", "_d1", ColumnType.STRING)
))
.setAggregatorSpecs(
aggregators(
@@ -6930,7 +6927,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
? new CountAggregatorFactory("_a0")
: new FilteredAggregatorFactory(
new CountAggregatorFactory("_a0"),
- not(selector("d1", null, null))
+ not(selector("d0", null, null))
)
)
)
@@ -10400,6 +10397,27 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
)
)
.setAggregatorSpecs(aggregators(new DoubleSumAggregatorFactory("a0", "m1")))
+ .setPostAggregatorSpecs(
+ ImmutableList.of(
+ expressionPostAgg(
+ "p0",
+ "timestamp_floor(\"d0\",'P1M',null,'UTC')"
+ )
+ )
+ )
+ .setHavingSpec(
+ having(
+ bound(
+ "a0",
+ "1",
+ null,
+ true,
+ false,
+ null,
+ StringComparators.NUMERIC
+ )
+ )
+ )
.setContext(
withTimestampResultContext(
QUERY_CONTEXT_DEFAULT,
@@ -10413,29 +10431,10 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
)
.setInterval(querySegmentSpec(Filtration.eternity()))
.setGranularity(Granularities.ALL)
- .setVirtualColumns(
- expressionVirtualColumn(
- "v0",
- "timestamp_floor(\"d0\",'P1M',null,'UTC')",
- ColumnType.LONG
- )
- )
.setDimensions(
dimensions(
new DefaultDimensionSpec("d1", "_d0"),
- new DefaultDimensionSpec("v0", "_d1", ColumnType.LONG)
- )
- )
- .setDimFilter(
- new BoundDimFilter(
- "a0",
- "1",
- null,
- true,
- null,
- null,
- null,
- StringComparators.NUMERIC
+ new DefaultDimensionSpec("p0", "_d1", ColumnType.LONG)
)
)
.setAggregatorSpecs(aggregators(new DoubleSumAggregatorFactory("_a0", "a0")))
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org