Skip to content

[CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join - #5301

Merged
zzwqqq merged 1 commit into
apache:mainfrom
ehds:fix-bushy-rule-bug
Oct 10, 2026
Merged

zzwqqq merged 1 commit into
apache:mainfrom
ehds:fix-bushy-rule-bug

Conversation

@ehds

@ehds ehds commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7833

Changes Proposed

The previous check relied on outerJoinFactors being non-empty, which misses FULL joins and cases such as LEFT JOIN ... ON TRUE, where the dependency set is empty.
This change checks isFullOuterJoin() and containsOuter() before optimizing.

@ehds ehds changed the title [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into … [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join Oct 3, 2026
sql(sql).withPre(preProgram).withProgram(program).check();
}

@Test void testBushyJoinRuleSkipsFullOuterJoin() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please add a comment referencing the jira above these test cases?

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.

Added the Jira reference to these test cases.

}

if (multiJoinRel.isFullOuterJoin() || multiJoinRel.containsOuter()) {
// Refuse to apply this rule to a multijoin with outer joins,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this rule rebuild all joins as INNER joins? If so, could you mention this in the comment?

@ehds ehds Oct 4, 2026 •

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.

Yes. This rule rebuilds all join factors from LoptMultiJoin as INNER joins, so it cannot preserve outer-join semantics.
The issue was frist reported by https://issues.apache.org/jira/browse/CALCITE-5289

@ehds
ehds force-pushed the fix-bushy-rule-bug branch from a0aaf34 to 2ec69bd Compare October 4, 2026 10:31
@zzwqqq zzwqqq added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 8, 2026
@zzwqqq

zzwqqq commented Oct 10, 2026

Copy link
Copy Markdown
Member

Please squash your commits @ehds

@ehds
ehds force-pushed the fix-bushy-rule-bug branch from 2ec69bd to 8b53906 Compare October 10, 2026 02:34
@ehds

ehds commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Please squash your commits @ehds

Done.

@ehds
ehds force-pushed the fix-bushy-rule-bug branch from 8b53906 to c885d02 Compare October 10, 2026 03:09
@sonarqubecloud

Copy link
Copy Markdown

@zzwqqq
zzwqqq merged commit b577854 into apache:main Oct 10, 2026
19 checks passed
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.

2 participants