Skip to content

[CALCITE-7885] RelDecorrelator merges distinct correlation fields and produces extra rows - #5353

Merged
mihaibudiu merged 1 commit into
apache:mainfrom
zzwqqq:fix/correlated-scalar-aggregate
Oct 23, 2026
Merged

mihaibudiu merged 1 commit into
apache:mainfrom
zzwqqq:fix/correlated-scalar-aggregate

Conversation

@zzwqqq

@zzwqqq zzwqqq commented Oct 23, 2026

Copy link
Copy Markdown
Member

Jira Link

CALCITE-7885

Changes Proposed

rewriteScalarAggregate uses aggregate column positions as keys in newProjectMap. Different outer references can share a position, so one overwrites another. When their values differ on unmatched rows, the final join can attach an aggregate result to the wrong outer row.

Rebuild the projection with correlation fields from valueGen, followed by the original aggregate outputs. Update corDefOutputs and outputMap to match the new positions, preserving each outer reference and COUNT's zero result for empty input.

@mihaibudiu

Copy link
Copy Markdown
Contributor

Since this is a bug which can produce incorrect results, I plan to merge it rather soon - given there is already an approval.

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 23, 2026
!ok
!set expand false

# [CALCITE-7885] RelDecorrelator merges distinct correlation fields and produces extra rows

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 checked this using Postgres.

@mihaibudiu

Copy link
Copy Markdown
Contributor

I will resolve the conflict and then hopefully merge once CI passes

@mihaibudiu
mihaibudiu force-pushed the fix/correlated-scalar-aggregate branch from 68fc01f to d513f59 Compare October 23, 2026 18:22
@mihaibudiu

Copy link
Copy Markdown
Contributor

Took the liberty to push the fix to your branch.

@sonarqubecloud

Copy link
Copy Markdown

@mihaibudiu
mihaibudiu merged commit c368970 into apache:main Oct 23, 2026
19 checks passed
@zzwqqq

zzwqqq commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Thanks @iwanttobepowerful and @mihaibudiu for reviewing and getting this merged!

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