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:
[email protected]


With regards,
Apache Git Services

Reply via email to