Skip to content

[CALCITE-7505] RelToSqlConverter fails to alias outer relation for correlated sub-queries in Filter #5036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dwrite wants to merge 5 commits into
base: main
Choose a base branch
from
+64 −10
Open

[CALCITE-7505] RelToSqlConverter fails to alias outer relation for correlated sub-queries in Filter #5036

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57d62e1
[CALCITE-7505] RelToSqlConverter produces duplicate FROM aliases for …
Jun 20, 2026
675a4ce
[CALCITE-7505] RelToSqlConverter fails to alias outer relation for co…
Jun 28, 2026
8801140
[CALCITE-7505] RelToSqlConverter fails to alias outer relation for co…
Jun 28, 2026
419bab8
[CALCITE-7505] RelToSqlConverter fails to alias outer relation for co…
Jun 28, 2026
df14a28
[CALCITE-7505] RelToSqlConverter fails to alias outer relation for co…
Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 108 additions & 2 deletions core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,15 @@
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
import org.apache.calcite.rex.RexCorrelVariable;
import org.apache.calcite.rex.RexInputRef;
import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexLocalRef;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexProgram;
import org.apache.calcite.rex.RexShuttle;
import org.apache.calcite.rex.RexSubQuery;
import org.apache.calcite.rex.RexVisitorImpl;
import org.apache.calcite.sql.JoinConditionType;
import org.apache.calcite.sql.JoinType;
import org.apache.calcite.sql.SqlAsofJoin;
Expand Down Expand Up @@ -547,9 +551,97 @@
return result(join, leftResult, rightResult);
}

/**
* Returns whether any expression (including nested sub-queries)
* references a correlated variable whose row type matches the given input.
*/
private static boolean inputRowTypeMatchesCorrelVariable(

Check failure on line 558 in core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 28 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ7qoS8Gtm-bIu2Ds3yZ&open=AZ7qoS8Gtm-bIu2Ds3yZ&pullRequest=5036
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)

@mihaibudiu mihaibudiu Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JavaDoc above says: "a correlated variable defined by the given input"
Where is this check validating this fact?
(I am not sure how "defined by an input" is defined)

@Dwrite Dwrite Jun 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry， that's my english fault, The wording is inaccurate. The method does not actually verify that the correlation variable is defined by the given input; it only checks for a correlated reference with a matching row type. I'll update the Javadoc to reflect what it actually does.

@mihaibudiu mihaibudiu Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be many variables with the same type, please explain why your approach is correct.

@Dwrite Dwrite Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mihaibudiu

Thanks for the question. During our test case execution, we observed that planner rules like FilterIntoJoinRule( test case with 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!

@mihaibudiu mihaibudiu Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me that you are describing a bug, I think having a wrong type on the correlated variables should be normally impossible - whoever rewrote the filter should have adjusted the variable types too. Am I wrong? If not, can you make sure that there is an issue filed for this?

Can a variable have the right type and also be obsolete at the same time?

Also, you can probably add an explanation here as a comment about why the type is needed.

@Dwrite Dwrite Jul 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, and I appreciate the sharp observation.

The rowType matching check is not a reliable discriminator. It was
introduced as an empirical workaround based on observed behavior in
specific test cases, not derived from a sound semantic invariant. As
you pointed out, a variable can have the correct type and still be
obsolete, so the check is not sound in general.

After mirroring the same variablesSet approach from Project to Filter,
all tests pass except for two cases that involve HepPlanner rules
(FilterIntoJoinRule-- testFilterCorrelateMissingVariableCor() and FilterCorrelateRule testFilterIntoJoinMissingVariableCor()
). I will investigate why
these two rules produce incorrect behavior and follow up.

@Dwrite Dwrite Jul 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mihaibudiu

I add new code !(definedHere.isEmpty()) && e.getInput().getInputs().size() <= 1. and here's explaination.
The purpose of calling resetAliasForCorrelation in visit(Filter) / visit(Project)
is narrow: give e.getInput() a canonical alias so that a correlated
sub-query embedded in this node's condition/projects can resolve
$corX.field against that alias.

This is only safe/necessary when e.getInput()'s rendered SQL already
represents a single addressable relation:

  • e.getInput().getInputs().size() <= 1 (TableScan, Values, Project,
    Filter, Aggregate, ...): these nodes always render as a single
    identity -- either a bare table reference or a freshly wrapped SELECT
    derived table. Assigning an alias here is safe, and is exactly the
    step the original bug was missing.

  • e.getInput().getInputs().size() > 1 (Join, Correlate): visit(Join)
    already builds a multi-alias context via
    joinContext(leftContext, rightContext) (e.g. {EMP: ..., DEPT: ...}),
    keeping each side independently addressable -- which is exactly what a
    correlated sub-query needs to resolve $corX.field. Calling
    resetAliasForCorrelation here would collapse that multi-alias context
    into a single alias, destroying field resolution that was already
    correct (this is the root cause of the DEPTNO0 leakage observed in
    the FilterIntoJoinRule / FilterCorrelateRule regression cases).

Scope: this check only governs whether visit(Filter) / visit(Project)
should invoke resetAliasForCorrelation for the purpose of correlation
variable binding. It says nothing about the general contract of
resetAliasForCorrelation itself, and does not affect other call sites
(e.g. visitAntiOrSemiJoin, which legitimately calls it on a
multi-alias Result for an unrelated purpose -- wrapping a complex
anti-join input). That is why the check belongs at this call site rather
than as an assertion inside resetAliasForCorrelation: an assertion
inside the shared method would incorrectly impose this call site's
precondition on every caller.

Verified against three concrete scenarios:

  • EXISTS in WHERE: e.getInput() = TableScan (0 inputs) -> pushed=true, correct
  • FilterIntoJoinRule / FilterCorrelateRule residual filter: e.getInput() = Join
    (2 inputs) -> pushed=false, correct (avoids DEPTNO0 leakage)
  • foodmart correlated IN, e.getInput() = Project over Join: Project itself
    has 1 input -> pushed=true, correct

&& v.getType().equals(inputRowType)) {
found = true;
}
return null;
}

@Override public Void visitSubQuery(RexSubQuery subQuery) {
if (!found && containsInRelTree(subQuery.rel)) {
found = true;
}
return found ? null : super.visitSubQuery(subQuery);
}

private boolean containsInRelTree(RelNode rel) {
if (found) {
return true;
}

rel.accept(new RexShuttle() {
@Override public RexNode visitCorrelVariable(
RexCorrelVariable v) {
if (correlIds.contains(v.id)
&& v.getType().equals(inputRowType)) {
found = true;
}
return v;
}

@Override public RexNode visitSubQuery(
RexSubQuery subQuery) {
if (!found) {
containsInRelTree(subQuery.rel);
}
return subQuery;
}
});

if (found) {

Check warning on line 616 in core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Change this condition so that it does not always evaluate to "false" 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ7qoS8Gtm-bIu2Ds3yY&open=AZ7qoS8Gtm-bIu2Ds3yY&pullRequest=5036
return true;
}

for (RelNode child : rel.getInputs()) {
if (containsInRelTree(child)) {
return true;
}
}

return false;
}
}

final Finder finder = new Finder();

for (RexNode expr : exprs) {
expr.accept(finder);
if (finder.found) {
return true;
}
}
return false;
}
/** Visits a Filter; called by {@link #dispatch} via reflection. */
public Result visit(Filter e) {

Check failure on line 641 in core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ7qoS8Gtm-bIu2Ds3ya&open=AZ7qoS8Gtm-bIu2Ds3ya&pullRequest=5036
final RelNode input = e.getInput();
final Set<CorrelationId> definedHere = e.getVariablesSet();

if (input instanceof Aggregate) {
final Aggregate aggregate = (Aggregate) input;
final boolean ignoreClauses = aggregate.getInput() instanceof Project;
Expand All @@ -564,8 +656,22 @@
return builder.result();
} else {
Result x = visitInput(e, 0, Clause.WHERE);
if (!e.getVariablesSet().isEmpty()) {
x = x.resetAlias();
final boolean pushed = !definedHere.isEmpty()
&& !(e.getInput() instanceof Join) && !(e.getInput() instanceof Correlate)
&& inputRowTypeMatchesCorrelVariable(
e.getInput(), definedHere, ImmutableList.of(e.getCondition()));
if (pushed) {
String alias = x.neededAlias;
if (alias != null) {
x = x.resetAliasForCorrelation(alias, e.getInput().getRowType());
} else {
alias = unqualifiedName(x.node);
if (alias == null) {
alias = "t";
}
x = x.resetAliasForCorrelation
(alias, e.getInput().getRowType());
}
}
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
Expand Down
37 changes: 31 additions & 6 deletions core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6527,7 +6527,7 @@ private void checkLiteral2(String expression, String expected) {
+ "from \"sales_fact_1997\"b "
+ "where b.\"product_id\" = a.\"product_id\")";
String expected = "SELECT \"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product\"\n"
+ "WHERE EXISTS (SELECT COUNT(*)\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "WHERE \"product_id\" = \"product\".\"product_id\")";
Expand All @@ -6540,7 +6540,7 @@ private void checkLiteral2(String expression, String expected) {
+ "from \"sales_fact_1997\"b "
+ "where b.\"product_id\" = a.\"product_id\")";
String expected = "SELECT \"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product\"\n"
+ "WHERE NOT EXISTS (SELECT COUNT(*)\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "WHERE \"product_id\" = \"product\".\"product_id\")";
Expand All @@ -6553,7 +6553,7 @@ private void checkLiteral2(String expression, String expected) {
+ "from \"sales_fact_1997\"b "
+ "where b.\"product_id\" = a.\"product_id\")";
String expected = "SELECT \"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product\"\n"
+ "WHERE \"product_id\" IN (SELECT \"product_id\"\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "WHERE \"product_id\" = \"product\".\"product_id\")";
Expand All @@ -6575,7 +6575,7 @@ private void checkLiteral2(String expression, String expected) {
+ "from \"sales_fact_1997\"b "
+ "where b.\"product_id\" = a.\"product_id\")";
String expected = "SELECT \"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product\"\n"
+ "WHERE \"product_id\" NOT IN (SELECT \"product_id\"\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "WHERE \"product_id\" = \"product\".\"product_id\")";
Expand All @@ -6593,7 +6593,7 @@ private void checkLiteral2(String expression, String expected) {
+ "where t2.\"product_id\" = t1.\"product_id\" "
+ "and t1.\"product_id\" = 2 and t2.\"product_id\" = 1)";
String expected = "SELECT \"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product\"\n"
+ "WHERE \"product_id\" NOT IN (SELECT \"product_id\"\n"
+ "FROM \"foodmart\".\"product\" AS \"product0\"\n"
+ "WHERE \"product_id\" = \"product\".\"product_id\" "
Expand Down Expand Up @@ -8892,7 +8892,7 @@ private void checkLiteral2(String expression, String expected) {
+ "GROUP BY \"t1\".\"department_id\"\n"
+ "HAVING \"t1\".\"department_id\" = MIN(\"t1\".\"department_id\")) \"t4\" ON \"employee\".\"department_id\" = \"t4\".\"department_id0\"";
final String expectedNoExpand = "SELECT \"department_id\"\n"
+ "FROM \"foodmart\".\"employee\"\n"
+ "FROM \"foodmart\".\"employee\" AS \"employee\"\n"
+ "WHERE \"department_id\" = (SELECT MIN(\"employee\".\"department_id\")\n"
+ "FROM \"foodmart\".\"department\"\n"
+ "WHERE 1 = 2)";
Expand Down Expand Up @@ -12324,4 +12324,29 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(query).withLibrary(SqlLibrary.HIVE).withHive().ok(expectedHive);
sql(query).withLibrary(SqlLibrary.SPARK).withSpark().ok(expectedSpark);
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7505">[CALCITE-7505]
* RelToSqlConverter fails to alias outer relation for correlated sub-queries in Filter</a>. */
@Test void testExistsSubQueryAliasConflict() {
Comment thread
Dwrite marked this conversation as resolved.
final String sql =
"select deptno, sum(sal) as total\n"
+ "from emp t\n"
+ "where exists (\n"
+ " select * from dept t0\n"
+ " where deptno = t.deptno\n"
+ ")\n"
+ "group by deptno";


sql(sql)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(
"SELECT \"DEPTNO\", SUM(\"SAL\") AS \"TOTAL\"\n"
+ "FROM \"SCOTT\".\"EMP\" AS \"EMP\"\n"
+ "WHERE EXISTS (SELECT *\n"
+ "FROM \"SCOTT\".\"DEPT\"\n"
+ "WHERE \"DEPTNO\" = \"EMP\".\"DEPTNO\")\n"
+ "GROUP BY \"DEPTNO\"");
}
}
Loading