You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "ehds (via GitHub)" <gi...@apache.org> on 2026/10/03 09:34:21 UTC

[PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into … [calcite]

ehds opened a new pull request, #5301:
URL: https://github.com/apache/calcite/pull/5301

   …an inner join
   
   <!--
   Thanks for sending a pull request! Here are some critical tips for you:
   
   1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing
      *For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.*
   
   2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira:
      https://issues.apache.org/jira/projects/CALCITE/issues
      *Check existing issues first to avoid duplicates.*
      *Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).*
   
   3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR).
      This gives others a chance to weigh in on your specification.
   
   4. CRITICAL CONSISTENCY RULE
      The following three items MUST match exactly in wording and meaning:
      (A) The Jira Issue Title
      (B) This Pull Request Title
      (C) Your Git Commit Message
   
      Format: [CALCITE-XXXX] <Description>
      Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE
   
      Guidelines for a good Title:
      - Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
      - Focus on the specification and user experience, not the implementation.
      - Make it clear whether it is a bug or a feature.
      - Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").
   
   5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review.
   
   6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated.
   -->
   
   ## Jira Link
   
   [CALCITE-7833](https://issues.apache.org/jira/browse/CALCITE-7833)
   
   ## Changes Proposed
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
   If possible, please consider writing useful notes for better and faster reviews in your PR.
   -->
   
   Prevent MULTI_JOIN_OPTIMIZE_BUSHY from rewriting outer joins as inner joins.
   The rule now skips FULL, LEFT, and RIGHT outer joins.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-6093362604

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5301:
URL: https://github.com/apache/calcite/pull/5301#discussion_r4177091141


##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -4656,6 +4656,20 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
     sql(sql).withPre(preProgram).withProgram(program).check();
   }
 
+  @Test void testBushyJoinRuleSkipsFullOuterJoin() {

Review Comment:
   Added the Jira reference to these test cases.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5301:
URL: https://github.com/apache/calcite/pull/5301#discussion_r4177107608


##########
core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java:
##########
@@ -104,16 +104,11 @@ public MultiJoinOptimizeBushyRule(RelFactories.JoinFactory joinFactory,
     final RelMetadataQuery mq = call.getMetadataQuery();
 
     final LoptMultiJoin multiJoin = new LoptMultiJoin(multiJoinRel);
-    for (int i = 0; i < multiJoin.getNumJoinFactors(); i++) {
-      ImmutableBitSet outerJoinFactors = multiJoin.getOuterJoinFactors(i);
-      if (outerJoinFactors == null) {
-        continue;
-      }
-      if (!outerJoinFactors.isEmpty()) {
-        // Refuse to apply this rule to a multijoin with outer joins,
-        // since this rule cannot handle outer joins.
-        return;
-      }
+
+    if (multiJoinRel.isFullOuterJoin() || multiJoinRel.containsOuter()) {
+      // Refuse to apply this rule to a multijoin with outer joins,

Review Comment:
   Yes. This rule rebuilds all join factors as INNER joins, so it cannot preserve outer-join semantics.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-5967992465

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-5979236529

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5301:
URL: https://github.com/apache/calcite/pull/5301#discussion_r4177107608


##########
core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java:
##########
@@ -104,16 +104,11 @@ public MultiJoinOptimizeBushyRule(RelFactories.JoinFactory joinFactory,
     final RelMetadataQuery mq = call.getMetadataQuery();
 
     final LoptMultiJoin multiJoin = new LoptMultiJoin(multiJoinRel);
-    for (int i = 0; i < multiJoin.getNumJoinFactors(); i++) {
-      ImmutableBitSet outerJoinFactors = multiJoin.getOuterJoinFactors(i);
-      if (outerJoinFactors == null) {
-        continue;
-      }
-      if (!outerJoinFactors.isEmpty()) {
-        // Refuse to apply this rule to a multijoin with outer joins,
-        // since this rule cannot handle outer joins.
-        return;
-      }
+
+    if (multiJoinRel.isFullOuterJoin() || multiJoinRel.containsOuter()) {
+      // Refuse to apply this rule to a multijoin with outer joins,

Review Comment:
   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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "zzwqqq (via GitHub)" <gi...@apache.org>.
zzwqqq merged PR #5301:
URL: https://github.com/apache/calcite/pull/5301


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-6093073959

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5301&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5301&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5301&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5301)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "zzwqqq (via GitHub)" <gi...@apache.org>.
zzwqqq commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-6092772063

   Please squash your commits @ehds 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on PR #5301:
URL: https://github.com/apache/calcite/pull/5301#issuecomment-6092809835

   > Please squash your commits @ehds
   
   Done.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7833] MULTI_JOIN_OPTIMIZE_BUSHY may turn an outer join into an inner join [calcite]

Posted by "zzwqqq (via GitHub)" <gi...@apache.org>.
zzwqqq commented on code in PR #5301:
URL: https://github.com/apache/calcite/pull/5301#discussion_r4175605105


##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -4656,6 +4656,20 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
     sql(sql).withPre(preProgram).withProgram(program).check();
   }
 
+  @Test void testBushyJoinRuleSkipsFullOuterJoin() {

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



##########
core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java:
##########
@@ -104,16 +104,11 @@ public MultiJoinOptimizeBushyRule(RelFactories.JoinFactory joinFactory,
     final RelMetadataQuery mq = call.getMetadataQuery();
 
     final LoptMultiJoin multiJoin = new LoptMultiJoin(multiJoinRel);
-    for (int i = 0; i < multiJoin.getNumJoinFactors(); i++) {
-      ImmutableBitSet outerJoinFactors = multiJoin.getOuterJoinFactors(i);
-      if (outerJoinFactors == null) {
-        continue;
-      }
-      if (!outerJoinFactors.isEmpty()) {
-        // Refuse to apply this rule to a multijoin with outer joins,
-        // since this rule cannot handle outer joins.
-        return;
-      }
+
+    if (multiJoinRel.isFullOuterJoin() || multiJoinRel.containsOuter()) {
+      // Refuse to apply this rule to a multijoin with outer joins,

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org