This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit f20b7c2a4a4b34347b1f37b851e41cb28e0a649f Author: kkasa <[email protected]> AuthorDate: Thu Aug 1 15:25:00 2024 +0200 [CALCITE-6513] FilterProjectTransposeRule may cause OOM when Project expressions are complex Close apache/calcite#3900 --- .../java/org/apache/calcite/plan/RelOptUtil.java | 22 +++++++++++++++ .../rel/rules/FilterProjectTransposeRule.java | 14 +++++++++- .../apache/calcite/rel/rules/ProjectMergeRule.java | 11 +++++--- .../java/org/apache/calcite/tools/RelBuilder.java | 2 +- .../org/apache/calcite/test/RelOptRulesTest.java | 31 ++++++++++++++++++++++ .../org/apache/calcite/test/RelOptRulesTest.xml | 26 ++++++++++++++++++ 6 files changed, 100 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java index 5387acd8cf..ffd1252e56 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java @@ -53,7 +53,9 @@ import org.apache.calcite.rel.logical.LogicalJoin; import org.apache.calcite.rel.logical.LogicalProject; import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.calcite.rel.rules.CoreRules; +import org.apache.calcite.rel.rules.FilterProjectTransposeRule; import org.apache.calcite.rel.rules.MultiJoin; +import org.apache.calcite.rel.rules.ProjectMergeRule; import org.apache.calcite.rel.stream.StreamRules; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; @@ -121,6 +123,7 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.Deque; import java.util.HashMap; @@ -148,6 +151,15 @@ public abstract class RelOptUtil { public static final double EPSILON = 1.0e-5; + /** Default amount by which the complexity of a {@link Project} or + * {@link Filter} may increase when applying a rule. (Complexity is, + * roughly, the number of {@link RexNode}s in all expressions.) + * + * @see ProjectMergeRule.Config#bloat() + * @see FilterProjectTransposeRule.Config#bloat() + * @see RelBuilder.Config#bloat() */ + public static final int DEFAULT_BLOAT = 100; + @SuppressWarnings("Guava") @Deprecated // to be removed before 2.0 public static final com.google.common.base.Predicate<Filter> @@ -3168,6 +3180,16 @@ public abstract class RelOptUtil { return pushShuttle(project).visitList(nodes); } + public static @Nullable RexNode pushPastProjectUnlessBloat(RexNode node, + Project project, int bloat) { + List<RexNode> newConditions = + pushPastProjectUnlessBloat(Collections.singletonList(node), project, bloat); + if (newConditions == null || newConditions.size() != 1) { + return null; + } + return newConditions.get(0); + } + /** As {@link #pushPastProject}, but returns null if the resulting expressions * are significantly more complex. * diff --git a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java index 5be607f821..726c07bb14 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java @@ -164,7 +164,10 @@ public class FilterProjectTransposeRule } // convert the filter to one that references the child of the project RexNode newCondition = - RelOptUtil.pushPastProject(filter.getCondition(), project); + RelOptUtil.pushPastProjectUnlessBloat(filter.getCondition(), project, config.bloat()); + if (newCondition == null) { + return; + } final RelBuilder relBuilder = call.builder(); RelNode newFilterRel; @@ -258,5 +261,14 @@ public class FilterProjectTransposeRule b2.operand(relClass).anyInputs()))) .as(Config.class); } + + /** Limit how much complexity can increase during merging. + * Default is {@link RelOptUtil#DEFAULT_BLOAT}. */ + @Value.Default default int bloat() { + return RelOptUtil.DEFAULT_BLOAT; + } + + /** Sets {@link #bloat()}. */ + Config withBloat(int bloat); } } diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java index 31f7e182a8..4608a8c8bc 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java @@ -45,8 +45,11 @@ public class ProjectMergeRule implements TransformationRule { /** Default amount by which complexity is allowed to increase. * - * @see Config#bloat() */ - public static final int DEFAULT_BLOAT = 100; + * @see Config#bloat() + * @deprecated please use {@link RelOptUtil#DEFAULT_BLOAT} + */ + @Deprecated // to be removed before 2.0 + public static final int DEFAULT_BLOAT = RelOptUtil.DEFAULT_BLOAT; /** Creates a ProjectMergeRule. */ protected ProjectMergeRule(Config config) { @@ -155,9 +158,9 @@ public class ProjectMergeRule } /** Limit how much complexity can increase during merging. - * Default is {@link #DEFAULT_BLOAT} (100). */ + * Default is {@link RelOptUtil#DEFAULT_BLOAT} (100). */ @Value.Default default int bloat() { - return ProjectMergeRule.DEFAULT_BLOAT; + return RelOptUtil.DEFAULT_BLOAT; } /** Sets {@link #bloat()}. */ diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java index 048012e57f..908915e48b 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -4905,7 +4905,7 @@ public class RelBuilder { * gather common sub-expressions and compute them only once. */ @Value.Default default int bloat() { - return 100; + return RelOptUtil.DEFAULT_BLOAT; } /** Sets {@link #bloat}. */ diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index be44def3b6..f9eb6dad23 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -2699,6 +2699,37 @@ class RelOptRulesTest extends RelOptTestBase { .check(); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6513";>[CALCITE-6513] + * FilterProjectTransposeRule may cause OOM when Project expressions are + * complex</a>. */ + @Test void testFilterProjectTransposeBloat() { + String sql = + "SELECT x1 from\n" + + " (SELECT 'L1' || x0 || x0 || x0 || x0 as x1 from\n" + + " (SELECT 'L0' || ENAME || ENAME || ENAME || ENAME as x0 from emp) t1) t2\n" + + "WHERE x1 = 'Something'"; + + final FilterProjectTransposeRule filterProjectTransposeRule = + CoreRules.FILTER_PROJECT_TRANSPOSE.config + .withOperandSupplier(b0 -> + b0.operand(Filter.class).predicate(filter -> true) + .oneInput(b1 -> + b1.operand(Project.class).predicate(project -> true) + .anyInputs())) + .as(FilterProjectTransposeRule.Config.class) + .withCopyFilter(true) + .withCopyProject(true) + .withBloat(3) + .toRule(); + sql(sql) + .withRelBuilderConfig(config -> config.withBloat(3)) + .withDecorrelate(false) + .withExpand(true) + .withRule(filterProjectTransposeRule) + .check(); + } + private static final String NOT_STRONG_EXPR = "case when e.sal < 11 then 11 else -1 * e.sal end"; diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml index b9b4e85aa6..a849461992 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -4607,6 +4607,32 @@ LogicalProject(EMPNO=[$0]) LogicalProject(TWICEDEPTNO=[*($0, 2)]) LogicalFilter(condition=[=($cor0.DEPTNO, *($0, 2))]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testFilterProjectTransposeBloat"> + <Resource name="sql"> + <![CDATA[SELECT x1 from + (SELECT 'L1' || x0 || x0 || x0 || x0 as x1 from + (SELECT 'L0' || ENAME || ENAME || ENAME || ENAME as x0 from emp) t1) t2 +WHERE x1 = 'Something']]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(X1=[$0]) + LogicalFilter(condition=[=($0, 'Something')]) + LogicalProject(X1=[||(||(||(||('L1', $0), $0), $0), $0)]) + LogicalProject(X0=[||(||(||(||('L0', $1), $1), $1), $1)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(X1=[$0]) + LogicalProject(X1=[||(||(||(||('L1', $0), $0), $0), $0)]) + LogicalFilter(condition=[=(||(||(||(||('L1', $0), $0), $0), $0), 'Something')]) + LogicalProject(X0=[||(||(||(||('L0', $1), $1), $1), $1)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> </Resource> </TestCase>
