kasakrisz commented on code in PR #3900:
URL: https://github.com/apache/calcite/pull/3900#discussion_r1702738770


##########
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:
   It is not a simple change: whether to merge `Projects` or not is controlled 
by `RelBuilder.Config.bloat()`, and the tests in `RelOptRulesTest` using 
`FnRelSupplier` create `RelBuilder` instances using a [constant 
config](https://github.com/apache/calcite/blob/979981481f8effde5e2fe5bb605d5c3424554e4b/testkit/src/main/java/org/apache/calcite/test/RelSupplier.java#L131).
   
   I tried to improve `RelSupplier` and `RelOptFixture.withRelBuilderConfig` to 
make `RelBuilder` configurable when `FnRelSupplier` is used. See: 
https://github.com/apache/calcite/pull/3900/commits/b8d1dce88672746e4f10925ef3298921c39c38c6
   
   Please let me know If I'm on the right track to tackle this.



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

Reply via email to