[GitHub] [calcite] hsyuan commented on a change in pull request #2429: [CALCITE-4630] Filter must not be pushed down to join children if there is limit expression in join children

[GitBox <gi...@apache.org>]

hsyuan commented on a change in pull request #2429:
URL: https://github.com/apache/calcite/pull/2429#discussion_r647835488



##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2860,9 +2862,10 @@ public static boolean classifyFilters(
         filtersToRemove.add(filter);
 
         // filters can be pushed to the right child if the right child
-        // does not generate NULLs and the only columns referenced in
-        // the filter originate from the right child
-      } else if (pushRight && rightBitmap.contains(inputBits)) {
+        // does not generate NULLs and does not contain limit and the only
+        // columns referenced in the filter originate from the right child
+      } else if (pushRight && rightBitmap.contains(inputBits)
+          && !containsLimit(joinRel.getInputs().get(1))) {

Review comment:
       use joinRel.getInput(1), joinRel.getInput(0) instead.

##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2904,6 +2907,33 @@ public static boolean classifyFilters(
     return !filtersToRemove.isEmpty();
   }
 
+  private static final RelVisitor LIMIT_FINDER = new RelVisitor() {
+    @Override public void visit(RelNode node, int ordinal, @Nullable RelNode parent) {
+      if (isLimit(node)) {
+        throw Util.FoundOne.NULL;
+      }
+
+      if (node instanceof HepRelVertex) {

Review comment:
       Can we avoid using HepRelVertex? I am afraid it might be planner specific. Have you tried to run the query on Volcano planner?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org