Skip to content

[CALCITE-7440] Preserve correlated scope mapping in RelToSqlConverter#4828

Draft
bvolpato wants to merge 2 commits intoapache:mainfrom
bvolpato:bvolpato/calcite-7440-rel2sql-correl-scope
Draft

[CALCITE-7440] Preserve correlated scope mapping in RelToSqlConverter#4828
bvolpato wants to merge 2 commits intoapache:mainfrom
bvolpato:bvolpato/calcite-7440-rel2sql-correl-scope

Conversation

@bvolpato
Copy link

@bvolpato bvolpato commented Mar 9, 2026

Summary

RelToSqlConverter failed during PostgreSQL round-trip validation when semi-join rewrites introduced a correlated filter with an unresolved correlation id.

Reproduction query

WITH product_keys AS (
  SELECT p."product_id",
         (SELECT MAX(p3."product_id")
          FROM "foodmart"."product" p3
          WHERE p3."product_id" = p."product_id") AS "mx"
  FROM "foodmart"."product" p
)
SELECT DISTINCT pk."product_id"
FROM product_keys pk
LEFT JOIN "foodmart"."product" p2 USING ("product_id")
WHERE pk."product_id" IN (
  SELECT p4."product_id"
  FROM "foodmart"."product" p4
)

Error before this PR

java.lang.NullPointerException: variable $cor1 is not found
  at org.apache.calcite.rel.rel2sql.SqlImplementor.getAliasContext(SqlImplementor.java:1590)

Fix

Register missing RexCorrelVariable ids while visiting filter conditions, without overriding existing correlation mappings.

Test

  • Added RelToSqlConverterTest.testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules
  • Ran: ./gradlew :core:test --tests org.apache.calcite.rel.rel2sql.RelToSqlConverterTest
  • Result: 563 passed, 0 failed

Disclaimer

The changes were done with the assistance of Codex (gpt-5.3-codex) in response to an actual bug when using Calcite in production. The changes were manually verified prior to sending this to review.

@bvolpato bvolpato force-pushed the bvolpato/calcite-7440-rel2sql-correl-scope branch 3 times, most recently from 6309f8d to 5447f29 Compare March 10, 2026 04:40
@bvolpato bvolpato marked this pull request as ready for review March 10, 2026 04:49
@bvolpato bvolpato marked this pull request as draft March 10, 2026 12:39
@bvolpato bvolpato force-pushed the bvolpato/calcite-7440-rel2sql-correl-scope branch 2 times, most recently from 4449f5a to f11eed6 Compare March 10, 2026 13:01
@bvolpato bvolpato marked this pull request as ready for review March 11, 2026 02:15
* <a href="https://issues.apache.org/jira/browse/CALCITE-7440">[CALCITE-7440]
* RelToSqlConverter throws NPE when correlation scope is missing after
* semi-join rewrites.</a>. */
@Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
Copy link
Member

Choose a reason for hiding this comment

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

I ran this test on the main branch, and strangely it passed. Could you check if your case can reproduce the problem you mentioned?

  @Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
    final String query = "WITH product_keys AS (\n"
        + "  SELECT p.\"product_id\",\n"
        + "         (SELECT MAX(p3.\"product_id\")\n"
        + "          FROM \"foodmart\".\"product\" p3\n"
        + "          WHERE p3.\"product_id\" = p.\"product_id\") AS \"mx\"\n"
        + "  FROM \"foodmart\".\"product\" p\n"
        + ")\n"
        + "SELECT DISTINCT pk.\"product_id\"\n"
        + "FROM product_keys pk\n"
        + "LEFT JOIN \"foodmart\".\"product\" p2 USING (\"product_id\")\n"
        + "WHERE pk.\"product_id\" IN (\n"
        + "  SELECT p4.\"product_id\"\n"
        + "  FROM \"foodmart\".\"product\" p4\n"
        + ")";

    final RuleSet rules = RuleSets.ofList();
    final String generated = sql(query).withPostgresql().optimize(rules, null).exec();
    sql(generated).withPostgresql().exec();
  }

Copy link
Member

Choose a reason for hiding this comment

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

Please take a look at my description; you may not have solved my problem.

@bvolpato
Copy link
Author

bvolpato commented Mar 13, 2026

Thanks for the review! Updated this PR with both requested changes:

Reduced the regression to the minimal reproducer on main (empty rules was a mistake):

  • FILTER_SUB_QUERY_TO_MARK_CORRELATE
  • PROJECT_SUB_QUERY_TO_MARK_CORRELATE
  • MARK_TO_SEMI_OR_ANTI_JOIN_RULE
  • SEMI_JOIN_JOIN_TRANSPOSE

Added a plain round-trip test without extra rules:

  • testPostgresqlRoundTripCorrelatedProject

Both new/updated tests pass locally in this branch.

@bvolpato bvolpato force-pushed the bvolpato/calcite-7440-rel2sql-correl-scope branch from 6365d37 to 565cb6a Compare March 13, 2026 12:10
@bvolpato bvolpato marked this pull request as draft March 13, 2026 12:32
@bvolpato bvolpato force-pushed the bvolpato/calcite-7440-rel2sql-correl-scope branch from 565cb6a to a685206 Compare March 13, 2026 12:50
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants