asolimando commented on code in PR #3900:
URL: https://github.com/apache/calcite/pull/3900#discussion_r1701822424
##########
core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java:
##########
@@ -258,5 +261,14 @@ default Config withOperandFor(Class<? extends Filter>
filterClass,
b2.operand(relClass).anyInputs())))
.as(Config.class);
}
+
+ /** Limit how much complexity can increase during merging.
+ * Default is {@link RelOptUtil#DEFAULT_BLOAT} (100). */
Review Comment:
Nit: Probably better to omit the value itself as one can easily look it up
with the IDE. The risk I see is that, in case the value gets changed in the
code, we end up with an inconsistency.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -2699,6 +2699,40 @@ private void
checkProjectCorrelateTransposeRuleSemiOrAntiCorrelate(JoinRelType t
.check();
}
+ /** Tests a variant of {@link FilterProjectTransposeRule}
+ * that pushes a Filter that contains a correlating variable. */
+ @Test void testFilterProjectTransposeBloat() {
+ final Function<RelBuilder, RelNode> relFn = b ->
+ b.scan("EMP")
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field("ENAME"),
b.field("ENAME")))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .project(b.call(SqlStdOperatorTable.CONCAT, b.field(0),
b.field(0)))
+ .filter(b.call(SqlStdOperatorTable.EQUALS, b.field(0),
b.literal("Something")))
+ .build();
+ 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(10)
Review Comment:
Can we use a smaller value and make the unit test code smaller? It's clear
what you want to show but it's a bit hard to follow in the current form IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
