Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram (#10120)

* Fix Stack overflow with SELECT ARRAY ['Hello', NULL]

* address comments
This commit is contained in:
Maytas Monsereenusorn 2020-07-01 17:48:09 -07:00 committed by GitHub
parent 477335abb4
commit 1676ba22e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 1 deletions

View File

@ -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

View File

@ -139,6 +139,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
{