Skip to content

[CALCITE-7753] CorrelateProjectExtractor corrupts plans with nested Correlates that reuse the same correlation id - #5231

Closed
snuyanzin wants to merge 1 commit into
apache:mainfrom
snuyanzin:calcite7753
Closed

snuyanzin wants to merge 1 commit into
apache:mainfrom
snuyanzin:calcite7753

Conversation

@snuyanzin

@snuyanzin snuyanzin commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7753

Changes Proposed

The change to fix CorrelateProjectExtractor for downstream projects

* <a href="https://issues.apache.org/jira/browse/CALCITE-7753">[CALCITE-7753]
* CorrelateProjectExtractor corrupts plans with nested Correlates that reuse
* the same correlation id</a>. */
@Test void testNestedCorrelatesSharingCorrelationId() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As mentioned in jira: it is not reproducible with pure SQL in Calcite

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because the bug requires two nested Correlates that share the same correlation id, and pure SQL (in Calcite) never produces that shape: Calcite's SqlToRelConverter gives every correlated subquery scope its own id.

@mihaibudiu

Copy link
Copy Markdown
Contributor

I left a comment related to this in Jira questioning whether this fix belongs in Calcite.

@mihaibudiu mihaibudiu left a comment •

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.

I guess the change is "safe", although the plan it rejects may be illegal.
But in general I don't think rules can guard against any possible kind of adversarial incorrect plan.

Comment thread core/src/main/java/org/apache/calcite/sql2rel/CorrelateProjectExtractor.java Outdated
@mihaibudiu

Copy link
Copy Markdown
Contributor

I guess is commit squash time!

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 6, 2026
… Correlates that reuse the same correlation id
@snuyanzin

Copy link
Copy Markdown
Contributor Author

yep, makes sense
squashed and rebased
will merge tomorrow if no objections

@snuyanzin

Copy link
Copy Markdown
Contributor Author

Merged as 25d62c5

@snuyanzin snuyanzin closed this Oct 7, 2026
@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

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

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants