rubenada commented on code in PR #3172:
URL: https://github.com/apache/calcite/pull/3172#discussion_r1174933805
##########
core/src/main/java/org/apache/calcite/rel/rules/SemiJoinJoinTransposeRule.java:
##########
@@ -102,16 +101,18 @@ public SemiJoinJoinTransposeRule(RelBuilderFactory
relBuilderFactory) {
// determine which operands below the semi-join are the actual
// Rels that participate in the semi-join
+ final ImmutableIntList leftKeys = semiJoin.analyzeCondition().leftKeys;
int nKeysFromX = 0;
for (int leftKey : leftKeys) {
if (leftKey < nFieldsX) {
nKeysFromX++;
}
}
- // the keys must all originate from either the left or right;
- // otherwise, a semi-join wouldn't have been created
- assert (nKeysFromX == 0) || (nKeysFromX == leftKeys.size());
+ if ((nKeysFromX != 0) && (nKeysFromX != leftKeys.size())) {
+ // We can not push semi-join down if join keys come from different
tables under the join.
Review Comment:
I'd suggest to make the comment a bit less ambiguous, e.g. as you described
in the Jira: "We cannot proceed if Semi-Join has keys from both tables of the
bottom Join".
Also, I think the Jira title (and hence the PR title and commit message)
should also be more specific, e.g.: "Assertion error in
SemiJoinJoinTransposeRule when Semi-Join has keys from both tables of the
bottom 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
