[GitHub] [calcite] chunweilei commented on a diff in pull request #2783: [CALCITE-5109] RelMdAllPredicates and RelMdExpressionLineage support to analyze left/right join

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

chunweilei commented on code in PR #2783:
URL: https://github.com/apache/calcite/pull/2783#discussion_r873709606


##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java:
##########
@@ -272,7 +302,55 @@
     if (allExprs == null) {
       return null;
     }
-    return newPreds.union(rexBuilder, RelOptPredicateList.of(rexBuilder, allExprs));
+
+    joinOnNewPreds = joinOnNewPreds.union(rexBuilder, RelOptPredicateList.of(rexBuilder, allExprs));
+    if (joinType == JoinRelType.LEFT) {
+      // Current join is left out join: left predicates could be pulled up, and right predicates
+      // and on predicates couldn't pull be pulled up.
+      ImmutableList<RexNode> joinOnPreds = wrapRefNullable(rexBuilder,
+          rightTableRefs, joinOnNewPreds.pulledUpPredicates);
+      ImmutableList<RexNode> rightAllPreds = ImmutableList.<RexNode>builder()
+          .addAll(joinOnPreds)
+          .addAll(rightNewPreds.pulledUpPredicates)
+          .build();
+      ImmutableList<RexNode> pullUpPreds = leftNewPreds.pulledUpPredicates;
+      return RelOptPredicateList.of(rexBuilder, pullUpPreds, ImmutableList.of(), rightAllPreds);
+    } else if (joinType == JoinRelType.RIGHT) {
+      // It's similar to left join.
+      // Current join is right out join: right predicates could be pulled up, and left predicates
+      // and on predicates couldn't pull be pulled up.
+      ImmutableList<RexNode> joinOnPreds = wrapRefNullable(rexBuilder,

Review Comment:
   > couldn't pull be pulled up
   
   Plz Remove redundant words.



##########
core/src/main/java/org/apache/calcite/rex/RexInputRef.java:
##########
@@ -94,6 +95,15 @@ public static RexInputRef of(int index, List<RelDataTypeField> fields) {
     return new RexInputRef(index, fields.get(index).getType());
   }
 
+  /**
+   * It's same to {@link RexInputRef#of(int, RelDataType)}, but it makes nullable reference.
+   */
+  public static RexInputRef ofNullable(RelDataTypeFactory factory, int index, RelDataType rowType) {
+    final List<RelDataTypeField> fields = rowType.getFieldList();

Review Comment:
   `It's ` is unnecessary.



##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java:
##########
@@ -272,7 +302,55 @@
     if (allExprs == null) {
       return null;
     }
-    return newPreds.union(rexBuilder, RelOptPredicateList.of(rexBuilder, allExprs));
+
+    joinOnNewPreds = joinOnNewPreds.union(rexBuilder, RelOptPredicateList.of(rexBuilder, allExprs));
+    if (joinType == JoinRelType.LEFT) {
+      // Current join is left out join: left predicates could be pulled up, and right predicates
+      // and on predicates couldn't pull be pulled up.
+      ImmutableList<RexNode> joinOnPreds = wrapRefNullable(rexBuilder,

Review Comment:
   > couldn't pull be pulled up
   
   Plz Remove redundant words.



##########
core/src/main/java/org/apache/calcite/rex/RexTableInputRef.java:
##########
@@ -46,10 +47,22 @@ public class RexTableInputRef extends RexInputRef {
 
   private final RelTableRef tableRef;
 
-  private RexTableInputRef(RelTableRef tableRef, int index, RelDataType type) {
+  /**
+   * It's a tag, which ref is wrapped by out join. And it always emits nullable = true.
+   * Input ref will be true, if it exists in right's input of left-join
+   * or left's input of right-join are true
+   */
+  private final boolean forceNullable;
+

Review Comment:
   The comment might be confusing to others. Could you reorganize it? Usually, we don't start with `It's`.



##########
core/src/main/java/org/apache/calcite/rex/RexTableInputRef.java:
##########
@@ -77,12 +91,28 @@ public int getIdentifier() {
     return tableRef.getEntityNumber();
   }
 
+  public boolean getForceNullable() {
+    return forceNullable;
+  }
+
   public static RexTableInputRef of(RelTableRef tableRef, int index, RelDataType type) {
-    return new RexTableInputRef(tableRef, index, type);
+    return new RexTableInputRef(tableRef, index, type, false);
   }
 
   public static RexTableInputRef of(RelTableRef tableRef, RexInputRef ref) {
-    return new RexTableInputRef(tableRef, ref.getIndex(), ref.getType());
+    return new RexTableInputRef(tableRef, ref.getIndex(), ref.getType(), false);
+  }
+
+  /**
+   * Make a {@link RexTableInputRef} with join's nullable, rebuild type's nullable info,
+   * if join's nullable is true.
+   */
+  public static RexTableInputRef of(RelDataTypeFactory factory, RelTableRef tableRef, int index,

Review Comment:
   Make -> Makes.
   
   > , rebuild type's nullable info, if join's nullable is true.
   
   It seems redundant.



##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -2405,6 +2409,78 @@ private void assertExpressionLineage(
     assertNull(r);
   }
 
+  @Test void testExpressionLineageLeftOuterJoin() {
+    // support to analyze for out join.
+    final RelNode rel = sql("select emp.sal as e_sal, dept.name as d_name\n"
+        + "from emp left outer join dept\n"

Review Comment:
   for out join > for left out join ?



##########
core/src/main/java/org/apache/calcite/rex/RexTableInputRef.java:
##########
@@ -77,12 +91,28 @@ public int getIdentifier() {
     return tableRef.getEntityNumber();
   }
 
+  public boolean getForceNullable() {
+    return forceNullable;
+  }

Review Comment:
   getForceNullable -> isForceNullable?



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