xuzifu666 commented on code in PR #5383:
URL: https://github.com/apache/calcite/pull/5383#discussion_r4476249412
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -3564,6 +3565,22 @@ protected RelNode createAsofJoin(
return node;
}
+ /** Returns the number of fields before a namespace in the FROM list of a
Review Comment:
`fieldOffset` returns 0 both when the namespace is the first item in the
FROM list and when it is not found at all, so the caller cannot distinguish the
two cases. If a future change ever produces a nested join whose
joinChildren.get(0) is not a direct child of ancestorScope,
requiredColumns.set(pos - offset) would silently use a wrong offset, producing
misaligned correlation column indices — the kind of bug that is hard to catch
in tests. Suggest returning -1 (or OptionalInt) when the namespace is not
found, and asserting at the call site to make the "cannot happen" invariant
explicit.
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -3644,10 +3678,11 @@ protected RelNode createAsofJoin(
throw new AssertionError("Identifier '" + lookup.originalRelName
+ "." + originalFieldName + "' is not a group expr");
}
- }
+ } // end for correlName
- requiredColumns.set(pos);
+ requiredColumns.set(pos - offset);
Review Comment:
Inside the loop, `correlRowType` is overwritten on every iteration, so the
last variable wins; but the `offset` > 0 branch afterwards builds fieldMapping
keyed by correlNames.get(0) and applies that row type via the shuttle. If the
join inputs reference two correlation variables from different sources (two
distinct CorrelationIds), the row type comes from the last variable's field
access while the rewrite uses the first variable's id. If this scenario is
reachable, the result would be wrong. Suggest adding assert correlNames.size()
== 1 in the offset > 0 branch, or handling each variable separately. if one
variable is truly the only possibility, the assert documents the invariant.
--
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]
