You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/09 15:59:17 UTC

[GitHub] [calcite] wojustme opened a new pull request #2013: [CALCITE-3935] Materialization-Failed, when querying with LeftJoinWithFilter

wojustme opened a new pull request #2013:
URL: https://github.com/apache/calcite/pull/2013


   


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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Materialization-Failed, when querying with LeftJoinWithFilter

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r446513799



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,

Review comment:
       Check if filter under join can be pulled up




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



[GitHub] [calcite] wojustme commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r699236578



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,

Review comment:
       ok, thanks for your remind.

##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterBottomJoin(JoinRelType joinType,
+                                                   RexNode leftFilterRexNode,
+                                                   RexNode rightFilterRexNode) {

Review comment:
       ok, I format it.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] jinxing64 commented on a change in pull request #2013: [CALCITE-3935] Materialization-Failed, when querying with LeftJoinWithFilter

Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r439321177



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1202,8 +1202,9 @@ private JoinOnLeftCalcToJoinUnifyRule() {
       if (joinRelType == null) {
         return null;
       }
-      if (joinRelType != JoinRelType.INNER
-          && !(joinRelType.isOuterJoin() && qInput0Cond.isAlwaysTrue())) {
+      // If JoinType is inner-join or out-join,
+      // Calc which is bottom join can be pulled up.
+      if (joinRelType != JoinRelType.INNER && !joinRelType.isOuterJoin()) {

Review comment:
       What if it's a right Join but with Calc as the left child ?
   If above concern is valid, please add a corresponding test.




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



[GitHub] [calcite] wojustme commented on pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-909158849


   @xy2953396112 
   Hi, Could take your time, review this pr?


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-921520550


   LGTM, trigger the checks again please.


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] wojustme commented on pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-917635486


   @yanlin-Lynn 
   Thanks for your advise.
   Please review again.


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn merged pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn merged pull request #2013:
URL: https://github.com/apache/calcite/pull/2013


   


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] wojustme commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r709892309



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2083,6 +2075,35 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check if filter under join can be pulled up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterUnderJoin(JoinRelType joinType,
+                                                  @Nullable RexNode leftFilterRexNode,
+                                                  @Nullable RexNode rightFilterRexNode) {

Review comment:
       @yanlin-Lynn 
   Thanks for reminding.
   Please review again.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] wojustme commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r699237017



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterBottomJoin(JoinRelType joinType,
+                                                   RexNode leftFilterRexNode,
+                                                   RexNode rightFilterRexNode) {

Review comment:
       ok, I format it.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r706774523



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1388,16 +1388,11 @@ private JoinOnCalcsToJoinUnifyRule() {
       final RexBuilder rexBuilder = call.getCluster().getRexBuilder();
 
       // Try pulling up MutableCalc only when:

Review comment:
       You should refine this note.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] wojustme commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r699236578



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,

Review comment:
       ok, thanks for your remind.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r706774858



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1303,8 +1303,8 @@ private JoinOnRightCalcToJoinUnifyRule() {
       if (joinRelType == null) {
         return null;
       }
-      if (joinRelType != JoinRelType.INNER
-          && !(joinRelType.isOuterJoin() && qInput1Cond.isAlwaysTrue())) {
+      // Check filter bottom join can be pulled up.

Review comment:
       ``` Check if filter under join can be pulled up```, keep the same with the note for this method.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] wojustme commented on pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-909158849


   @xy2953396112 
   Hi, Could take your time, review this pr?


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Materialization-Failed, when querying with LeftJoinWithFilter

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r446513643



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterBottomJoin(JoinRelType joinType,
+                                                   RexNode leftFilterRexNode,
+                                                   RexNode rightFilterRexNode) {

Review comment:
       we do not format like 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.

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



[GitHub] [calcite] wojustme commented on pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-909158849


   @xy2953396112 
   Hi, Could take your time, review this pr?


-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r706775411



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2083,6 +2078,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check if filter under join can be pulled up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterUnderJoin(JoinRelType joinType,
+                                                  @Nullable RexNode leftFilterRexNode,
+                                                  @Nullable RexNode rightFilterRexNode) {
+    if (joinType == JoinRelType.INNER) {
+      return true;
+    }
+    if (joinType == JoinRelType.LEFT) {
+      if (rightFilterRexNode == null || rightFilterRexNode.isAlwaysTrue()) {

Review comment:
       Suggest you write like this.
   ```
   if (joinType == JoinRelType.LEFT) && (rightFilterRexNode == null || rightFilterRexNode.isAlwaysTrue()) {
     return true;
   }
   ```




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up filters under join of left or right (xurenhe)

Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r707929544



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2083,6 +2075,35 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check if filter under join can be pulled up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterUnderJoin(JoinRelType joinType,
+                                                  @Nullable RexNode leftFilterRexNode,
+                                                  @Nullable RexNode rightFilterRexNode) {

Review comment:
       fix this indentation please.




-- 
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: commits-unsubscribe@calcite.apache.org

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



[GitHub] [calcite] jinxing64 commented on pull request #2013: [CALCITE-3935] Materialization-Failed, when querying with LeftJoinWithFilter

Posted by GitBox <gi...@apache.org>.
jinxing64 commented on pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#issuecomment-643206255


   Please refine the commit message.


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



[GitHub] [calcite] wojustme commented on a change in pull request #2013: [CALCITE-3935] Enhance Join-Materialization, support to pull-up bottom filters of `Left/Right` join (xurenhe)

Posted by GitBox <gi...@apache.org>.
wojustme commented on a change in pull request #2013:
URL: https://github.com/apache/calcite/pull/2013#discussion_r699236578



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,

Review comment:
       ok, thanks for your remind.

##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -2011,6 +2006,38 @@ public static boolean equalType(String desc0, MutableRel rel0, String desc1,
     return RelOptUtil.equal(desc0, rel0.rowType, desc1, rel1.rowType, litmus);
   }
 
+  /**
+   * Check filter bottom join can be pull-up,
+   * when meeting JoinOnCalc of query unify to Join of target.
+   * Such as {@link JoinOnLeftCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnRightCalcToJoinUnifyRule} <br/>
+   * {@link JoinOnCalcsToJoinUnifyRule} <br/>
+   */
+  private static boolean canPullUpFilterBottomJoin(JoinRelType joinType,
+                                                   RexNode leftFilterRexNode,
+                                                   RexNode rightFilterRexNode) {

Review comment:
       ok, I format it.




-- 
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: commits-unsubscribe@calcite.apache.org

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