rubenada commented on code in PR #4750:
URL: https://github.com/apache/calcite/pull/4750#discussion_r2701066249
##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -1953,51 +1946,98 @@ private static boolean isWidening(RelDataType type,
RelDataType type1) {
}
Frame newLeftFrame = leftFrame;
+ Frame newRightFrame = rightFrame;
+ final NavigableMap<CorDef, Integer> leftCorDefOutputs = new TreeMap<>();
+ final NavigableMap<CorDef, Integer> rightCorDefOutputs = new TreeMap<>();
boolean joinConditionContainsFieldAccess =
RexUtil.containsFieldAccess(rel.getCondition());
- if (joinConditionContainsFieldAccess && isCorVarDefined) {
- final CorelMap localCorelMap = new CorelMapBuilder().build(rel);
- final List<CorRef> corVarList = new
ArrayList<>(localCorelMap.mapRefRelToCorRef.values());
- Collections.sort(corVarList);
+ boolean generatesNullsOnRight = rel.getJoinType().generatesNullsOnRight();
+ boolean generatesNullsOnLeft = rel.getJoinType().generatesNullsOnLeft();
+
+ final CorelMap localCorelMap = new CorelMapBuilder().build(rel);
+ final List<CorRef> corVarList = new
ArrayList<>(localCorelMap.mapRefRelToCorRef.values());
+ Collections.sort(corVarList);
+
+ if ((joinConditionContainsFieldAccess || generatesNullsOnRight) &&
isCorVarDefined) {
+ newLeftFrame =
+ createFrameWithValueGenerator(oldLeft, leftFrame, corVarList,
leftCorDefOutputs);
+ }
- final NavigableMap<CorDef, Integer> corDefOutputs = new TreeMap<>();
- newLeftFrame = createFrameWithValueGenerator(oldLeft, leftFrame,
corVarList, corDefOutputs);
+ if (generatesNullsOnLeft && isCorVarDefined) {
+ newRightFrame =
+ createFrameWithValueGenerator(oldRight, rightFrame, corVarList,
rightCorDefOutputs);
}
+ // Build join conditions: original condition + correlated variable
conditions
+ final List<RexNode> joinConditions = new ArrayList<>();
+
+ // Add the original join condition
+ RexNode originalCond = decorrelateExpr(castNonNull(currentRel), map, cm,
rel.getCondition());
+ if (!originalCond.isAlwaysTrue()) {
+ joinConditions.add(originalCond);
+ }
+
+ int newLeftFieldCount = newLeftFrame.r.getRowType().getFieldCount();
+
+ // For joins that generate nulls on the right (LEFT, FULL, LEFT_ASOF),
+ // add conditions to connect correlated variables from left and right
+ if (generatesNullsOnRight && !leftCorDefOutputs.isEmpty()) {
+ // Iterate over preserved right side, lookup value generators on
nullable left side
+ final List<RexNode> conds =
+ buildCorDefJoinConditions(newRightFrame.corDefOutputs,
leftCorDefOutputs,
+ newLeftFrame.r, newRightFrame.r, newLeftFieldCount, relBuilder,
true);
+ joinConditions.addAll(conds);
+ }
+
+ // For joins that generate nulls on the left (RIGHT, FULL),
+ // add conditions to connect correlated variables from left and right
+ if (generatesNullsOnLeft && !rightCorDefOutputs.isEmpty()) {
+ // Iterate over preserved left side, lookup value generators on nullable
right side
+ final List<RexNode> conds =
+ buildCorDefJoinConditions(newLeftFrame.corDefOutputs,
rightCorDefOutputs,
+ newLeftFrame.r, newRightFrame.r, newLeftFieldCount, relBuilder,
false);
+ joinConditions.addAll(conds);
+ }
+
+ RexNode finalCondition = joinConditions.isEmpty()
+ ? relBuilder.literal(true)
+ : RexUtil.composeConjunction(relBuilder.getRexBuilder(),
joinConditions);
+
RelNode newJoin = relBuilder
.push(newLeftFrame.r)
- .push(rightFrame.r)
- .join(rel.getJoinType(),
- decorrelateExpr(castNonNull(currentRel), map, cm,
rel.getCondition()),
- ImmutableSet.of())
+ .push(newRightFrame.r)
+ .join(rel.getJoinType(), finalCondition, ImmutableSet.of())
.build();
- // Create the mapping between the output of the old correlation rel
- // and the new join rel
- Map<Integer, Integer> mapOldToNewOutputs = new HashMap<>();
-
int oldLeftFieldCount = oldLeft.getRowType().getFieldCount();
- int newLeftFieldCount = newLeftFrame.r.getRowType().getFieldCount();
-
int oldRightFieldCount = oldRight.getRowType().getFieldCount();
//noinspection AssertWithSideEffects
assert rel.getRowType().getFieldCount()
== oldLeftFieldCount + oldRightFieldCount;
+ // Create the mapping between the output of the old correlation rel and
the new join rel
// Left input positions are not changed.
- mapOldToNewOutputs.putAll(newLeftFrame.oldToNewOutputs);
+ Map<Integer, Integer> mapOldToNewOutputs = new
HashMap<>(newLeftFrame.oldToNewOutputs);
// Right input positions are shifted by newLeftFieldCount.
for (int i = 0; i < oldRightFieldCount; i++) {
mapOldToNewOutputs.put(i + oldLeftFieldCount,
- requireNonNull(rightFrame.oldToNewOutputs.get(i)) +
newLeftFieldCount);
+ requireNonNull(newRightFrame.oldToNewOutputs.get(i)) +
newLeftFieldCount);
}
- final NavigableMap<CorDef, Integer> corDefOutputs =
- new TreeMap<>(newLeftFrame.corDefOutputs);
+ final NavigableMap<CorDef, Integer> corDefOutputs = new
TreeMap<>(newLeftFrame.corDefOutputs);
// Right input positions are shifted by newLeftFieldCount.
- for (Map.Entry<CorDef, Integer> entry
- : rightFrame.corDefOutputs.entrySet()) {
- corDefOutputs.put(entry.getKey(),
- entry.getValue() + newLeftFieldCount);
+ for (Map.Entry<CorDef, Integer> entry :
newRightFrame.corDefOutputs.entrySet()) {
+ if (generatesNullsOnRight) {
+ // For joins that generate nulls on the right (LEFT, FULL, LEFT_ASOF),
+ // prefer left side corDef (from value generator) because right side
might be NULL
+ corDefOutputs.putIfAbsent(entry.getKey(), entry.getValue() +
newLeftFieldCount);
+ } else if (generatesNullsOnLeft) {
+ // For joins that generate nulls on the left (RIGHT, FULL),
Review Comment:
nit: The code in the `else if` block and the `else` look the same, maybe we
could combine them? (keeping both comments for clarity)
--
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]
