Dwrite commented on code in PR #5036:
URL: https://github.com/apache/calcite/pull/5036#discussion_r3487939870
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -547,9 +552,97 @@ public Result visit(Correlate e) {
return result(join, leftResult, rightResult);
}
+ /**
+ * Returns whether any expression (including nested sub-queries) references
a correlated variable
+ * defined by the given input and having the same row type.
+ */
+ private static boolean inputRowTypeMatchesCorrelVariable(
+ RelNode input,
+ Set<CorrelationId> correlIds,
+ Iterable<? extends RexNode> exprs) {
+
+ if (correlIds.isEmpty()) {
+ return false;
+ }
+
+ final RelDataType inputRowType = input.getRowType();
+
+ /** Visitor that detects matching correlated variables. */
+ class Finder extends RexVisitorImpl<Void> {
+ boolean found;
+
+ Finder() {
+ super(true);
+ }
+
+ @Override public Void visitCorrelVariable(RexCorrelVariable v) {
+ if (correlIds.contains(v.id)
Review Comment:
Hi @mihaibudiu
Thanks for the question. During our test case execution, we observed that
planner rules like FilterIntoJoinRule(testFilterCorrelateMissingVariableCor)
can reposition or push down a Filter. This optimization often causes the
Filter's variablesSet to become stale, meaning the variable's rowType no longer
matches the schema of the new underlying input.
Therefore, relying solely on correlIds.contains(v.id) is insufficient. We
need the rowType equality check to ensure that this Filter remains the true
binding point of the correlation variable post-optimization. If the types
mismatch, it indicates the Filter has been relocated, and we should not trigger
resetAliasForCorrelation.
Regarding the structure check, rather than a generic BiRel, I specifically
restrict this validation to Join and LogicalCorrelate operators. These two
operators represent the exact semantic boundaries where correlation aliases are
defined and resolved. Checking for Join and LogicalCorrelate allows us to
correctly determine whether the alias needs to be appended, ensuring
correctness without interfering with other relational nodes.
Let me know if this clarifies the design choice!
--
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]
