[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r288784931
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3739,12 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality,
+   * this mostly depends on the physical implementation of the join. */
+  public static boolean forceEquiJoin(Join join) {
+return join.isSemiJoin() || join instanceof EquiJoin;
 
 Review comment:
   @danny0405 Can you mark method `forceEquiJoin` subject to change or remove? 
I view it as a temporary walk around if I don't get it wrong.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r288784197
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
 ##
 @@ -116,6 +115,15 @@ protected FilterJoinRule(RelOptRuleOperand operand, 
String id,
 
   //~ Methods 
 
+  /** Returns if it is needed to push the filter condition above join
+   * into the join condition.
+   */
+  private boolean needsPushInto(Join join) {
+// If the join force the join info to be based on column equality,
+// Or it is a non-correlated semijoin, returns false.
 
 Review comment:
   @danny0405 Can you update the comment?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-23 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r287176675
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
 ##
 @@ -116,6 +115,15 @@ protected FilterJoinRule(RelOptRuleOperand operand, 
String id,
 
   //~ Methods 
 
+  /** Returns if it is needed to push the filter condition above join
+   * into the join condition.
+   */
+  private boolean needsPushInto(Join join) {
+// If the join force the join info to be based on column equality,
+// Or it is a non-correlated semijoin, returns false.
 
 Review comment:
   A SemiJoin can't be correlated.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-23 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r287176590
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3739,12 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality,
+   * this mostly depends on the physical implementation of the join. */
+  public static boolean forceEquiJoin(Join join) {
+return join.isSemiJoin() || join instanceof EquiJoin;
 
 Review comment:
   That is a wrong assumption. SemiJoin's join condition doesn't need to be 
equi condition.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285290481
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java
 ##
 @@ -186,7 +189,8 @@ protected RelMdExpressionLineage() {}
 // Extract input fields referenced by expression
 final ImmutableBitSet inputFieldsUsed = extractInputRefs(outputExpression);
 
-if (rel.getJoinType() != JoinRelType.INNER) {
+if (rel.getJoinType().generatesNullsOnLeft()
+|| rel.getJoinType().generatesNullsOnRight()) {
 
 Review comment:
   Can we use `isOuterJoin` to do the check? Or just remove this check, add 
`FULL` check in `else` in L209.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285290030
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMdDistinctRowCount.java
 ##
 @@ -140,35 +139,13 @@ public Double getDistinctRowCount(Filter rel, 
RelMetadataQuery mq,
 
   public Double getDistinctRowCount(Join rel, RelMetadataQuery mq,
   ImmutableBitSet groupKey, RexNode predicate) {
-if (predicate == null || predicate.isAlwaysTrue()) {
-  if (groupKey.isEmpty()) {
-return 1D;
-  }
-}
 return RelMdUtil.getJoinDistinctRowCount(mq, rel, rel.getJoinType(),
 groupKey, predicate, false);
   }
 
   public Double getDistinctRowCount(SemiJoin rel, RelMetadataQuery mq,
 
 Review comment:
   Let's deprecate 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285231506
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/EquiJoin.java
 ##
 @@ -27,12 +27,33 @@
 
 /**
  * Base class for any join whose condition is based on column equality.
+ *
+ * For most of the cases, {@link JoinInfo#isEqui()} can already decide
+ * if the join condition is based on column equality.
+ *
+ * {@code EquiJoin} is an abstract class for inheritance of Calcite 
enumerable
+ * joins and join implementation of other system. You should inherit the 
{@code EquiJoin}
+ * if your join implementation does not support non-equi join conditions. 
Calcite would
+ * eliminate some optimize logic for {@code EquiJoin} in some planning rules.
+ * e.g. {@link org.apache.calcite.rel.rules.FilterJoinRule} would not push 
non-equi
+ * join conditions of the above filter into the join underneath if it is an 
{@code EquiJoin}.
  */
 public abstract class EquiJoin extends Join {
 
 Review comment:
   We should also deprecate `EquiJoin`.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285296030
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3739,12 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality,
+   * this mostly depends on the physical implementation of the join. */
+  public static boolean forceEquiJoin(Join join) {
+return join.isSemiJoin() || join instanceof EquiJoin;
 
 Review comment:
   Now you can remove `forceEquiJoin ` and just use `return 
join.analyzeCondition().isEqui();`


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285293098
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
 ##
 @@ -619,6 +620,7 @@ public RelOptPredicateList inferPredicates(
 
   final RexBuilder rexBuilder = joinRel.getCluster().getRexBuilder();
   switch (joinType) {
+  case SEMI:
 
 Review comment:
   L627~629 should be pulled up here. SEMI can separate with INNER.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285230529
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
 ##
 @@ -400,13 +398,16 @@ public RelNode createCorrelate(RelNode left, RelNode 
right,
   /**
* Implementation of {@link SemiJoinFactory} that returns a vanilla
* {@link SemiJoin}.
+   *
+   * @deprecated Use {@link JoinFactoryImpl} instead.
*/
+  @Deprecated  // to be removed before 2.0
   private static class SemiJoinFactoryImpl implements SemiJoinFactory {
 public RelNode createSemiJoin(RelNode left, RelNode right,
-RexNode condition) {
+  RexNode condition) {
 
 Review comment:
   change back the indent.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285291567
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java
 ##
 @@ -258,11 +262,17 @@ protected RelMdExpressionLineage() {}
 }
 // Right input references might need to be updated if there are
 // table names clashes with left input
+final RelDataType fullRowType = SqlValidatorUtil.createJoinType(
+rexBuilder.getTypeFactory(),
+rel.getLeft().getRowType(),
+rel.getRight().getRowType(),
+null,
+ImmutableList.of());
 
 Review comment:
   Can you explain 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285229897
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
 ##
 @@ -43,6 +91,14 @@ public boolean generatesNullsOnLeft() {
 return (this == RIGHT) || (this == FULL);
   }
 
+  /**
+   * Returns whether a join of this type may generate NULL values, either on 
the
+   * left-hand side or right-hand side.
+   */
+  public boolean generatesNulls() {
 
 Review comment:
   Can we just name it to `isOuterJoin`?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-17 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r285230725
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
 ##
 @@ -400,13 +398,16 @@ public RelNode createCorrelate(RelNode left, RelNode 
right,
   /**
* Implementation of {@link SemiJoinFactory} that returns a vanilla
* {@link SemiJoin}.
+   *
+   * @deprecated Use {@link JoinFactoryImpl} instead.
*/
+  @Deprecated  // to be removed before 2.0
   private static class SemiJoinFactoryImpl implements SemiJoinFactory {
 public RelNode createSemiJoin(RelNode left, RelNode right,
-RexNode condition) {
+  RexNode condition) {
   final JoinInfo joinInfo = JoinInfo.of(left, right, condition);
   return SemiJoin.create(left, right,
-condition, joinInfo.leftKeys, joinInfo.rightKeys);
+  condition, joinInfo.leftKeys, joinInfo.rightKeys);
 
 Review comment:
   and here


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-01 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r280252142
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java
 ##
 @@ -659,15 +736,12 @@ public static Double getJoinRowCount(RelMetadataQuery 
mq, Join join,
 }
 double product = left * right;
 
-// TODO:  correlation factor
 return product * mq.getSelectivity(join, condition);
   }
 
   /** Returns an estimate of the number of rows returned by a
* {@link SemiJoin}. */
-  public static Double getSemiJoinRowCount(RelMetadataQuery mq, RelNode left,
-  RelNode right, JoinRelType joinType, RexNode condition) {
-// TODO:  correlation factor
+  public static Double getSemiJoinRowCount(RelMetadataQuery mq, RelNode left, 
RexNode condition) {
 
 Review comment:
   Why do you change the function signature? The right and joinType is not used 
for now, but doesn't mean we don't use it in the future. The current 
implementation is pretty naive.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-01 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r280246687
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   I would recommend inline `forceEquiJoin` and remove this function, because 
it seems like a temporary walk around  for this refactoring. Exposing this as a 
function makes us hard to remove it in future release.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-05-01 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r280246038
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   Sorry, I don't get it. What's your plan on `EquiJoin`? Keep it or deprecate 
it in another patch?
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-30 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279996918
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   I would rather trust Oracle's documentation about `EquiJoin`: 
https://docs.oracle.com/cd/E11882_01/server.112/e41084/queries006.htm#SQLRF52350
   
   However, it is meaningless and pedantic to argue what is an EquiJoin. Think 
about this question. What makes the so-called `EquiJoin` special? Is it because 
we can create hash join or merge join for it? If yes, the `EquiJoin` should be 
removed. As long as there is an equality join condition, both sides of the 
equality operator contain columns from single table, and there is no subquery 
in the condition, and both sides are hashable or sortable, even it is not a 
so-called `EquiJoin`, we can still create hash join or merge join for it. The 
current `EquiJoin` in Calcite has so many limitations. Even though we can make 
additional project to make each side become a single column to satisfy 
`EquiJoin`, it will add another operator, adding additional execution cost.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-30 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279983857
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   In fact, we don't need EquiJoin at all. The definition of EquiJoin in 
Calcite is misleading.  
   `t1.a+t1.b = t2.c+t2.d` would not be a EquiJoin. What we care about is does 
it have equal condition that we can do hash or merge join on it. But given the 
size of change, I don't mind to remove/deprecate EquiJoin in later patches.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-30 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279615583
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   > this method returns what the original EquiJoin is, cause now the 
join.analyzeCondition().isEqui() has wider scope that this method returns. 
   
   What do you mean by wider scope? 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279618398
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
 ##
 @@ -94,6 +142,37 @@ public JoinRelType cancelNullsOnRight() {
   return this;
 }
   }
+
+  /** Transform this JoinRelType to CorrelateJoinType. **/
+  public CorrelateJoinType toLinq4j() {
+switch (this) {
+case INNER:
+  return CorrelateJoinType.INNER;
+case LEFT:
+  return CorrelateJoinType.LEFT;
+case SEMI:
+  return CorrelateJoinType.SEMI;
+case ANTI:
+  return CorrelateJoinType.ANTI;
+}
+throw new IllegalStateException(
+"Unable to convert " + this + " to CorrelateJoinType");
+  }
 
 Review comment:
   Do we need this function? Can't we just pass JoinRelType directly?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279615811
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   We should never call enumerable classes here, because enumerable adapter may 
not be used by other system, most likely.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279587903
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
 ##
 @@ -71,26 +71,25 @@
 EnumerableRules.LOGGER.debug(e.toString());
 return null;
   }
+} else {
+  RelNode newRel;
+  try {
+newRel = EnumerableHashJoin.create(
+left,
+right,
+info.getEquiCondition(left, right, cluster.getRexBuilder()),
 
 Review comment:
   This call is so redundant, when we create a join, it should have everything 
that is related with join info. Ideally, we should just call 
`join.getJoinInfo().getEquiCondition()` or just `join.getEquiCondition()`. But 
we can do it later.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279586072
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
 ##
 @@ -76,13 +76,13 @@ public static EnumerableCorrelate create(
 
   @Override public EnumerableCorrelate copy(RelTraitSet traitSet,
   RelNode left, RelNode right, CorrelationId correlationId,
-  ImmutableBitSet requiredColumns, SemiJoinType joinType) {
+  ImmutableBitSet requiredColumns, JoinRelType joinType) {
 return new EnumerableCorrelate(getCluster(),
 traitSet, left, right, correlationId, requiredColumns, joinType);
   }
 
   public Result implement(EnumerableRelImplementor implementor,
-  Prefer pref) {
+  Prefer pref) {
 
 Review comment:
   Can you change the indent back?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-29 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279228408
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
 ##
 @@ -161,31 +142,48 @@ public static EnumerableJoin create(
 } else {
   rowCount += rightRowCount;
 }
-return planner.getCostFactory().makeCost(rowCount, 0, 0);
+if (isNonCorrelatedSemiJoin()) {
 
 Review comment:
   variablesSet should always be empty in Join.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279240531
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableThetaJoin.java
 ##
 @@ -45,6 +46,7 @@
 /** Implementation of {@link org.apache.calcite.rel.core.Join} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}
  * that allows conditions that are not just {@code =} (equals). */
+@Deprecated // to be removed before 2.0
 
 Review comment:
   Our common agreement is Rename EnumerableThetaJoin to NestedLoopJoin, and 
keep EnumerableCorrelate unchanged. If you do it in another patch, better 
revert the change for EnumerableCorrelate, don't rename 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279236014
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableThetaJoin.java
 ##
 @@ -45,6 +46,7 @@
 /** Implementation of {@link org.apache.calcite.rel.core.Join} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}
  * that allows conditions that are not just {@code =} (equals). */
+@Deprecated // to be removed before 2.0
 
 Review comment:
   @danny0405 Can we do above in this patch?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279228078
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   We can do it in a different patch.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279228408
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
 ##
 @@ -161,31 +142,48 @@ public static EnumerableJoin create(
 } else {
   rowCount += rightRowCount;
 }
-return planner.getCostFactory().makeCost(rowCount, 0, 0);
+if (isNonCorrelatedSemiJoin()) {
 
 Review comment:
   variablesSet should always be empty in Join.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279218729
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
 ##
 @@ -161,31 +142,48 @@ public static EnumerableJoin create(
 } else {
   rowCount += rightRowCount;
 }
-return planner.getCostFactory().makeCost(rowCount, 0, 0);
+if (isNonCorrelatedSemiJoin()) {
 
 Review comment:
   It won't be correlated, even in the future.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279214406
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   Correlate's condition is always TRUE, which is expected. What do you mean by 
`eliminate it's ability.`?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-28 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279182010
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   variablesSet is not used in Join, and shouldn't be used. I agree with you, 
no need to have variablesSet in Join operator.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-27 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279158905
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableThetaJoin.java
 ##
 @@ -45,6 +46,7 @@
 /** Implementation of {@link org.apache.calcite.rel.core.Join} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}
  * that allows conditions that are not just {@code =} (equals). */
+@Deprecated // to be removed before 2.0
 
 Review comment:
   Yes, we can rename EnumerableThetaJoin to EnumerableNestedLoopJoin, and keep 
EnumerableCorrelate as the LogicalCorrelate's physical implementation, to make 
things easier.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-27 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279158703
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   NestedLoop can represent both Join and Correlate. Say for query
   ```
   select * from R, S where r1 > s1;
   ```
   
   We can have plan:
   ```
   NLJ (r1 > s1)
+- R
+- S
   
   for (r in R) {
 for (s in S) {
   if (condition(r, s) is true)
output r,s;
 }
   }
   ```
   
   or 
   ```
   NLJ (true)
 +- R
 +- Filter (r1 > s1)
   +- S
   
   for (r in R) {
 while (s = scanNext(innerRel, r))
output r,s;
   }
   ```
   
   The difference is can we start fetching inner tuple without knowing the 
tuple from outer. It is impossible for the 2nd, which is a Correlate 
implementation. It is especially true if the inner is a index scan.
   
   It is true that we can always use the second one to implement it, depending 
on the implementation, that's why we have JoinToCorrelate rule.
   
   But logically I think we'd better not mix them.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-27 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279157499
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   @danny0405 The purpose of transforming a correlate to a join is getting rid 
of Variables.
   @zabetak It is in RelDecorrelator, which is not rule based, sadly. Some 
SubqueryRemoveRule will transform subquery into Correlate, some 
SubqueryRemoveRule will directly transform subquery into a Join. :(


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279139370
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   A join should not be correlated. Only `Correlate` is allowed to be 
correlated, in theory.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279130911
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
 ##
 @@ -71,26 +71,25 @@
 EnumerableRules.LOGGER.debug(e.toString());
 return null;
   }
+} else {
+  RelNode newRel;
+  try {
+newRel = EnumerableHashJoin.create(
+left,
+right,
+info.getEquiCondition(left, right, cluster.getRexBuilder()),
+join.getVariablesSet(),
+join.getJoinType());
+  } catch (InvalidRelException e) {
+EnumerableRules.LOGGER.debug(e.toString());
+return null;
+  }
+  if (!info.isEqui()) {
+newRel = new EnumerableFilter(cluster, newRel.getTraitSet(),
+newRel, info.getRemaining(cluster.getRexBuilder()));
+  }
 
 Review comment:
   I know you didn't change the logic here, but I can smell something fishy 
about the creation of the filter.
   The columns used by the filter may not be outputted by join, we have to do 
additional work like adjusting output columns in Join, adding Project
   like
   ```
   select r1, s1 from R, S where r2=s2 and r3 > s3;
   ```
   
   In fact, I am wondering do we need the EquiJoinInfo and NonEquiJoinInfo. We 
just need JoinInfo telling us equi conditions and non-equi conditions, whether 
it **has** equi or not. Will changing JoinInfo to a concrete class and 
deprecating EquiJoinInfo/NonEquiJoinInfo better serve us?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279139253
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
 ##
 @@ -347,7 +346,7 @@ public JoinReduceExpressionsRule(Class 
joinClass,
   matchNullability)) {
 return;
   }
-  if (join instanceof EquiJoin) {
+  if (RelOptUtil.forceEquiJoin(join)) {
 
 Review comment:
   I didn't get this. Can you explain this if block?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279127223
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
 ##
 @@ -161,31 +142,48 @@ public static EnumerableJoin create(
 } else {
   rowCount += rightRowCount;
 }
-return planner.getCostFactory().makeCost(rowCount, 0, 0);
+if (isNonCorrelatedSemiJoin()) {
 
 Review comment:
   Do we need to check it is correlated or not? It is definitely not 
correlated, right?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279139370
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   A join should not be correlated. Only Correlated is allowed to be 
correlated, in theory.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-26 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r279127354
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
 ##
 @@ -161,31 +142,48 @@ public static EnumerableJoin create(
 } else {
   rowCount += rightRowCount;
 }
-return planner.getCostFactory().makeCost(rowCount, 0, 0);
+if (isNonCorrelatedSemiJoin()) {
+  return planner.getCostFactory().makeCost(rowCount, 0, 
0).multiplyBy(.01d);
 
 Review comment:
   Why multiply by 0.01?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-18 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276558022
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   The addition of this method confuses me.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

2019-04-18 Thread GitBox
hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276553183
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   This looks weird.


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:
us...@infra.apache.org


With regards,
Apache Git Services