rubenada 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_r279667744
##########
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:
If I am not mistaken, the logic of this rule is:
```
if (join condition is not equi-join AND join is not INNER)
create EnumerableThetaJoin (now EnumerableNestedLoopJoin)
else
create EnumerableJoin (now EnumereableHashJoin) with the equi-join part of
the condition
if (join condition is not equi-join) // i.e. it's an inner join, therefore
all fields are available for the filter
add EnumerableFilter with the non-equi-join part of the condition
```
Thus, the filter can only be added for non-equi parts of an inner join, so I
think this change, which keeps the same logic, is correct.
However, with this PR, we will theoretically have new scenarios:
- Non-equi SEMI and ANTI joins will go through EnumerableThetaJoin (now
EnumerableNestedLoopJoin). If I am not mistaken, I think SEMI might work fine
(to be verified), but ANTI will not work as expected, so probably an
UnsupportedOperationException should be thrown in that case.
- Equi SEMI and ANTI joins will go through EnumerableJoin (now
EnumereableHashJoin). SEMI will be supported (via BuiltInMethod.SEMI_JOIN), but
ANTI will not be, so either we throw an UnsupportedOperationException or we
implement it (a new BuiltInMethod.ANTI_JOIN could be simply a copy-paste of
BuiltInMethod.SEMI_JOIN with one difference `return
innerLookup.get().contains(key);` becomes `return
!innerLookup.get().contains(key);` ). But this could be implemented as a
separate PR, and for the moment the UnsupportedOperationException could be
enough.
----------------------------------------------------------------
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
