This is an automated email from the ASF dual-hosted git repository.
jonwei 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 1676ba2 Fix Stack overflow with infinite loop in
ReduceExpressionsRule of HepProgram (#10120)
1676ba2 is described below
commit 1676ba22e300ea95cc92c0808f390aaa769546f9
Author: Maytas Monsereenusorn <mayt...@apache.org>
AuthorDate: Wed Jul 1 17:48:09 2020 -0700
Fix Stack overflow with infinite loop in ReduceExpressionsRule of
HepProgram (#10120)
* Fix Stack overflow with SELECT ARRAY ['Hello', NULL]
* address comments
---
.../apache/druid/sql/calcite/planner/Rules.java | 30 +++++++++++++++++++++-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 13 ++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
index 03b1a31..c9135d5 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
@@ -26,10 +26,13 @@ import org.apache.calcite.plan.RelOptMaterialization;
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.plan.hep.HepProgramBuilder;
import org.apache.calcite.plan.volcano.AbstractConverter;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.metadata.DefaultRelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMetadataProvider;
import org.apache.calcite.rel.rules.AggregateCaseToFilterRule;
import org.apache.calcite.rel.rules.AggregateExpandDistinctAggregatesRule;
import org.apache.calcite.rel.rules.AggregateJoinTransposeRule;
@@ -85,6 +88,16 @@ public class Rules
public static final int DRUID_CONVENTION_RULES = 0;
public static final int BINDABLE_CONVENTION_RULES = 1;
+ // Due to Calcite bug (CALCITE-3845), ReduceExpressionsRule can considered
expression which is the same as the
+ // previous input expression as reduced. Basically, the expression is
actually not reduced but is still considered as
+ // reduced. Hence, this resulted in an infinite loop of Calcite trying to
reducing the same expression over and over.
+ // Calcite 1.23.0 fixes this issue by not consider expression as reduced if
this case happens. However, while
+ // we are still using Calcite 1.21.0, a workaround is to limit the number of
pattern matches to avoid infinite loop.
+ private static final String HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING =
"druid.sql.planner.hepMatchLimit";
+ private static final int HEP_DEFAULT_MATCH_LIMIT = Integer.valueOf(
+ System.getProperty(HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING, "1200")
+ );
+
// Rules from RelOptUtil's registerBaseRules, minus:
//
// 1) AggregateExpandDistinctAggregatesRule (it'll be added back later if
approximate count distinct is disabled)
@@ -191,12 +204,14 @@ public class Rules
public static List<Program> programs(final PlannerContext plannerContext,
final QueryMaker queryMaker)
{
+
+
// Program that pre-processes the tree before letting the full-on
VolcanoPlanner loose.
final Program preProgram =
Programs.sequence(
Programs.subQuery(DefaultRelMetadataProvider.INSTANCE),
DecorrelateAndTrimFieldsProgram.INSTANCE,
- Programs.hep(REDUCTION_RULES, true,
DefaultRelMetadataProvider.INSTANCE)
+ buildHepProgram(REDUCTION_RULES, true,
DefaultRelMetadataProvider.INSTANCE, HEP_DEFAULT_MATCH_LIMIT)
);
return ImmutableList.of(
@@ -205,6 +220,19 @@ public class Rules
);
}
+ private static Program buildHepProgram(Iterable<? extends RelOptRule> rules,
+ boolean noDag,
+ RelMetadataProvider metadataProvider,
+ int matchLimit)
+ {
+ final HepProgramBuilder builder = HepProgram.builder();
+ builder.addMatchLimit(matchLimit);
+ for (RelOptRule rule : rules) {
+ builder.addRuleInstance(rule);
+ }
+ return Programs.of(builder.build(), noDag, metadataProvider);
+ }
+
private static List<RelOptRule> druidConventionRuleSet(
final PlannerContext plannerContext,
final QueryMaker queryMaker
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 17c970f..483cdd2 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
@@ -140,6 +140,19 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
}
@Test
+ public void testExpressionContainingNull() throws Exception
+ {
+ List<String> expectedResult = new ArrayList<>();
+ expectedResult.add("Hello");
+ expectedResult.add(null);
+ testQuery(
+ "SELECT ARRAY ['Hello', NULL]",
+ ImmutableList.of(),
+ ImmutableList.of(new Object[]{expectedResult})
+ );
+ }
+
+ @Test
public void testSelectNonNumericNumberLiterals() throws Exception
{
// Tests to convert NaN, positive infinity and negative infinity as
literals.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
