From 0f67e86edccf2240e73f2df567b96c684c6a716d Mon Sep 17 00:00:00 2001 From: sviatahorau Date: Mon, 8 Apr 2024 13:54:25 +0200 Subject: [PATCH 1/5] Configurable bloat for calcite ProjectMergeRule implemented --- .../druid/sql/calcite/planner/CalciteRulesManager.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index e7b909b5327c..e2ee1dc35812 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -38,6 +38,7 @@ import org.apache.calcite.rel.rules.CoreRules; import org.apache.calcite.rel.rules.DateRangeRules; import org.apache.calcite.rel.rules.JoinPushThroughJoinRule; +import org.apache.calcite.rel.rules.ProjectMergeRule; import org.apache.calcite.rel.rules.PruneEmptyRules; import org.apache.calcite.sql.SqlExplainFormat; import org.apache.calcite.sql.SqlExplainLevel; @@ -82,6 +83,10 @@ public class CalciteRulesManager private static final int HEP_DEFAULT_MATCH_LIMIT = Integer.parseInt( System.getProperty(HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING, "1200") ); + private static final String BLOAT_PROPERTY = "druid.sql.planner.bloat"; + private static final int BLOAT = Integer.parseInt( + System.getProperty(BLOAT_PROPERTY, "100") + ); /** * Rules from {@link org.apache.calcite.plan.RelOptRules#BASE_RULES}, minus: @@ -100,7 +105,7 @@ public class CalciteRulesManager ImmutableList.of( CoreRules.AGGREGATE_STAR_TABLE, CoreRules.AGGREGATE_PROJECT_STAR_TABLE, - CoreRules.PROJECT_MERGE, + ProjectMergeRule.Config.DEFAULT.withBloat(BLOAT).toRule(), CoreRules.FILTER_SCAN, CoreRules.FILTER_PROJECT_TRANSPOSE, CoreRules.JOIN_PUSH_EXPRESSIONS, From 368a655fb54adcee5f0fe204f4fd7940eac098d5 Mon Sep 17 00:00:00 2001 From: sviatahorau Date: Tue, 9 Apr 2024 10:24:09 +0200 Subject: [PATCH 2/5] Comment added --- .../apache/druid/sql/calcite/planner/CalciteRulesManager.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index e2ee1dc35812..9258bee4b724 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -100,6 +100,10 @@ public class CalciteRulesManager * and {@link CoreRules#FILTER_INTO_JOIN}, which are part of {@link #FANCY_JOIN_RULES}. * 4) {@link CoreRules#PROJECT_FILTER_TRANSPOSE} because PartialDruidQuery would like to have the Project on top of the Filter - * this rule could create a lot of non-useful plans. + * + * {@link CoreRules#PROJECT_MERGE} includes configurable bloat parameter, as a workaround for Calcite exception + * (there are not enough rules to produce a node with desired properties) thrown while running complex sql-queries with + * big amount of subqueries. `druid.sql.planner.bloat` should be set in broker's `jvm.config` file. */ private static final List BASE_RULES = ImmutableList.of( From 9f599eee24dc93f0a62b68610e9e56c5d9121eda Mon Sep 17 00:00:00 2001 From: sviatahorau Date: Wed, 10 Apr 2024 17:15:05 +0200 Subject: [PATCH 3/5] Default bloat value increased to 1000 --- .../apache/druid/sql/calcite/planner/CalciteRulesManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index 9258bee4b724..e5381c3cc1f0 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -85,7 +85,7 @@ public class CalciteRulesManager ); private static final String BLOAT_PROPERTY = "druid.sql.planner.bloat"; private static final int BLOAT = Integer.parseInt( - System.getProperty(BLOAT_PROPERTY, "100") + System.getProperty(BLOAT_PROPERTY, "1000") ); /** From f4c9109622aceb251d0f2a45bab1ca24c37eef63 Mon Sep 17 00:00:00 2001 From: sviatahorau Date: Mon, 22 Apr 2024 14:47:21 +0200 Subject: [PATCH 4/5] Implemented bloat configuration from QueryContext --- .../calcite/planner/CalciteRulesManager.java | 22 +++++--- .../planner/CalcitePlannerModuleTest.java | 51 +++++++++++++++++++ 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index e5381c3cc1f0..fac428652563 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -83,10 +83,8 @@ public class CalciteRulesManager private static final int HEP_DEFAULT_MATCH_LIMIT = Integer.parseInt( System.getProperty(HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING, "1200") ); - private static final String BLOAT_PROPERTY = "druid.sql.planner.bloat"; - private static final int BLOAT = Integer.parseInt( - System.getProperty(BLOAT_PROPERTY, "1000") - ); + public static final String BLOAT_PROPERTY = "sql.planner.bloat"; + public static final int DEFAULT_BLOAT = 1000; /** * Rules from {@link org.apache.calcite.plan.RelOptRules#BASE_RULES}, minus: @@ -100,16 +98,14 @@ public class CalciteRulesManager * and {@link CoreRules#FILTER_INTO_JOIN}, which are part of {@link #FANCY_JOIN_RULES}. * 4) {@link CoreRules#PROJECT_FILTER_TRANSPOSE} because PartialDruidQuery would like to have the Project on top of the Filter - * this rule could create a lot of non-useful plans. - * - * {@link CoreRules#PROJECT_MERGE} includes configurable bloat parameter, as a workaround for Calcite exception + * 5) {@link CoreRules#PROJECT_MERGE} added later with bloat parameter configured from query context as a workaround for Calcite exception * (there are not enough rules to produce a node with desired properties) thrown while running complex sql-queries with - * big amount of subqueries. `druid.sql.planner.bloat` should be set in broker's `jvm.config` file. + * big amount of subqueries. */ private static final List BASE_RULES = ImmutableList.of( CoreRules.AGGREGATE_STAR_TABLE, CoreRules.AGGREGATE_PROJECT_STAR_TABLE, - ProjectMergeRule.Config.DEFAULT.withBloat(BLOAT).toRule(), CoreRules.FILTER_SCAN, CoreRules.FILTER_PROJECT_TRANSPOSE, CoreRules.JOIN_PUSH_EXPRESSIONS, @@ -444,6 +440,15 @@ public List bindableConventionRuleSet(final PlannerContext plannerCo .build(); } + public List configurableRuleSet(PlannerContext plannerContext){ + return ImmutableList.of(ProjectMergeRule.Config.DEFAULT.withBloat(getBloatProperty(plannerContext)).toRule()); + } + + private int getBloatProperty(PlannerContext plannerContext) { + final Integer bloat = plannerContext.queryContext().getInt(BLOAT_PROPERTY); + return (bloat != null) ? bloat : DEFAULT_BLOAT; + } + public List baseRuleSet(final PlannerContext plannerContext) { final PlannerConfig plannerConfig = plannerContext.getPlannerConfig(); @@ -453,6 +458,7 @@ public List baseRuleSet(final PlannerContext plannerContext) rules.addAll(BASE_RULES); rules.addAll(ABSTRACT_RULES); rules.addAll(ABSTRACT_RELATIONAL_RULES); + rules.addAll(configurableRuleSet(plannerContext)); if (plannerContext.getJoinAlgorithm().requiresSubquery()) { rules.addAll(FANCY_JOIN_RULES); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java index 06d8cf761ab8..b1262a94bff7 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java @@ -30,6 +30,7 @@ import org.apache.calcite.plan.RelOptRule; import org.apache.calcite.plan.RelOptRuleCall; import org.apache.calcite.rel.logical.LogicalTableScan; +import org.apache.calcite.rel.rules.ProjectMergeRule; import org.apache.calcite.schema.Schema; import org.apache.druid.guice.LazySingleton; import org.apache.druid.jackson.DefaultObjectMapper; @@ -61,10 +62,13 @@ import javax.validation.Validator; import java.util.Collections; +import java.util.Optional; import java.util.Set; import static org.apache.calcite.plan.RelOptRule.any; import static org.apache.calcite.plan.RelOptRule.operand; +import static org.apache.druid.sql.calcite.planner.CalciteRulesManager.BLOAT_PROPERTY; +import static org.apache.druid.sql.calcite.planner.CalciteRulesManager.DEFAULT_BLOAT; @ExtendWith(EasyMockExtension.class) public class CalcitePlannerModuleTest extends CalciteTestBase @@ -72,6 +76,7 @@ public class CalcitePlannerModuleTest extends CalciteTestBase private static final String SCHEMA_1 = "SCHEMA_1"; private static final String SCHEMA_2 = "SCHEMA_2"; private static final String DRUID_SCHEMA_NAME = "DRUID_SCHEMA_NAME"; + private static final int BLOAT = 1200; @Mock private NamedSchema druidSchema1; @@ -204,4 +209,50 @@ public void testExtensionCalciteRule() .contains(customRule); Assert.assertTrue(containsCustomRule); } + + @Test + public void testConfigurableBloat() + { + ObjectMapper mapper = new DefaultObjectMapper(); + PlannerToolbox toolbox = new PlannerToolbox( + injector.getInstance(DruidOperatorTable.class), + macroTable, + mapper, + injector.getInstance(PlannerConfig.class), + rootSchema, + joinableFactoryWrapper, + CatalogResolver.NULL_RESOLVER, + "druid", + new CalciteRulesManager(ImmutableSet.of()), + CalciteTests.TEST_AUTHORIZER_MAPPER, + AuthConfig.newBuilder().build() + ); + + PlannerContext contextWithBloat = PlannerContext.create( + toolbox, + "SELECT 1", + new NativeSqlEngine(queryLifecycleFactory, mapper), + Collections.singletonMap(BLOAT_PROPERTY, BLOAT), + null + ); + + PlannerContext contextWithoutBloat = PlannerContext.create( + toolbox, + "SELECT 1", + new NativeSqlEngine(queryLifecycleFactory, mapper), + Collections.emptyMap(), + null + ); + + assertBloat(contextWithBloat, BLOAT); + assertBloat(contextWithoutBloat, DEFAULT_BLOAT); + } + + private void assertBloat(PlannerContext context, int expectedBloat) { + Optional firstProjectMergeRule = injector.getInstance(CalciteRulesManager.class).baseRuleSet(context).stream() + .filter(rule -> rule instanceof ProjectMergeRule) + .map(rule -> (ProjectMergeRule) rule) + .findAny(); + Assert.assertTrue(firstProjectMergeRule.isPresent() && firstProjectMergeRule.get().config.bloat() == expectedBloat); + } } From 8cd5f37e53ec71b535a66aabdcabe4f9d1a5baaf Mon Sep 17 00:00:00 2001 From: sviatahorau Date: Mon, 22 Apr 2024 15:46:09 +0200 Subject: [PATCH 5/5] Code refactored, docs updated --- docs/querying/query-context.md | 1 + .../druid/sql/calcite/planner/CalciteRulesManager.java | 8 +++++--- .../sql/calcite/planner/CalcitePlannerModuleTest.java | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/querying/query-context.md b/docs/querying/query-context.md index 98d9c9c6aa77..d5ddea04f27d 100644 --- a/docs/querying/query-context.md +++ b/docs/querying/query-context.md @@ -65,6 +65,7 @@ See [SQL query context](sql-query-context.md) for other query context parameters |`secondaryPartitionPruning`|`true`|Enable secondary partition pruning on the Broker. The Broker will always prune unnecessary segments from the input scan based on a filter on time intervals, but if the data is further partitioned with hash or range partitioning, this option will enable additional pruning based on a filter on secondary partition dimensions.| |`debug`| `false` | Flag indicating whether to enable debugging outputs for the query. When set to false, no additional logs will be produced (logs produced will be entirely dependent on your logging level). When set to true, the following addition logs will be produced:
- Log the stack trace of the exception (if any) produced by the query | |`setProcessingThreadNames`|`true`| Whether processing thread names will be set to `queryType_dataSource_intervals` while processing a query. This aids in interpreting thread dumps, and is on by default. Query overhead can be reduced slightly by setting this to `false`. This has a tiny effect in most scenarios, but can be meaningful in high-QPS, low-per-segment-processing-time scenarios. | +|`sqlPlannerBloat`|`1000`|Calcite parameter which controls whether to merge two Project operators when inlining expressions causes complexity to increase. Implemented as a workaround to exception `There are not enough rules to produce a node with desired properties: convention=DRUID, sort=[]` thrown after rejecting the merge of two projects.| ## Parameters by query type diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index fac428652563..e35011ba8bba 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -83,7 +83,7 @@ public class CalciteRulesManager private static final int HEP_DEFAULT_MATCH_LIMIT = Integer.parseInt( System.getProperty(HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING, "1200") ); - public static final String BLOAT_PROPERTY = "sql.planner.bloat"; + public static final String BLOAT_PROPERTY = "sqlPlannerBloat"; public static final int DEFAULT_BLOAT = 1000; /** @@ -440,11 +440,13 @@ public List bindableConventionRuleSet(final PlannerContext plannerCo .build(); } - public List configurableRuleSet(PlannerContext plannerContext){ + public List configurableRuleSet(PlannerContext plannerContext) + { return ImmutableList.of(ProjectMergeRule.Config.DEFAULT.withBloat(getBloatProperty(plannerContext)).toRule()); } - private int getBloatProperty(PlannerContext plannerContext) { + private int getBloatProperty(PlannerContext plannerContext) + { final Integer bloat = plannerContext.queryContext().getInt(BLOAT_PROPERTY); return (bloat != null) ? bloat : DEFAULT_BLOAT; } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java index b1262a94bff7..8ef3ad3106f3 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitePlannerModuleTest.java @@ -248,7 +248,8 @@ public void testConfigurableBloat() assertBloat(contextWithoutBloat, DEFAULT_BLOAT); } - private void assertBloat(PlannerContext context, int expectedBloat) { + private void assertBloat(PlannerContext context, int expectedBloat) + { Optional firstProjectMergeRule = injector.getInstance(CalciteRulesManager.class).baseRuleSet(context).stream() .filter(rule -> rule instanceof ProjectMergeRule) .map(rule -> (ProjectMergeRule) rule)