You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "wq352400523 (via GitHub)" <gi...@apache.org> on 2023/08/19 11:43:58 UTC

[GitHub] [calcite] wq352400523 opened a new pull request, #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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

   (no 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


[GitHub] [calcite] wq352400523 commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   The initial plan is :
   ```
   LogicalProject(deptno=[$0], added=[$1], name=[$2], empid=[$3], deptno0=[$4], name0=[$5], deptno1=[$6], added0=[$7], name1=[$8], empid0=[$9], deptno2=[$10], name2=[$11])
     LogicalJoin(condition=[=($6, $10)], joinType=[inner])
       LogicalJoin(condition=[=($0, $6)], joinType=[inner])
         LogicalJoin(condition=[=($0, $4)], joinType=[inner])
           LogicalTableScan(table=[[depts]])
           LogicalTableScan(table=[[emps]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
   ```
   
   And the whole best plan after this rule is :
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalJoin(condition=[=($3, $0)], joinType=[inner])
         LogicalTableScan(table=[[depts]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   
   You can see that 'depts' now become a self-join after rule, and join-key of this self-join is unique key , then we can remove this join further, like:
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   It doesn't mean to just simply remove the join , but instead with it child relNode , because they are equivalent.



-- 
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


[GitHub] [calcite] LakeShen commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""
+        + "LogicalJoin(condition=[=($3, $0)], joinType=[inner])\n"

Review Comment:
   I have saw the emps  self join was removed.I have question is that why self join could be removed when join key is unique,and I think you could also add this explanation to the code comments to make it easier for others to understand.
   
   If the field of emps is selected in the select statement, I think emps could not be removed, is it right?
   



-- 
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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3379)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL)
   
   [![60.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '60.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list) [60.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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-5927] LoptOptimizeJoinRule miss removable self-join when col… [calcite]

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

   #4097 fixed this issue. Thanks for @wq352400523 's contribution. I will close this PR.


-- 
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


[GitHub] [calcite] wq352400523 commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""
+        + "LogicalJoin(condition=[=($3, $0)], joinType=[inner])\n"

Review Comment:
   Actually it doesn't remove self-join now in LoptOptimizeJoinRule, it just reorder the join factors to find removable self-join and put them together. After that , we can remove this self-join further in our custom optimize rule. My response to your next commit may help to understand.
   And thanks for your review!



##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   The initial plan is :
   ```
   LogicalProject(deptno=[$0], added=[$1], name=[$2], empid=[$3], deptno0=[$4], name0=[$5], deptno1=[$6], added0=[$7], name1=[$8], empid0=[$9], deptno2=[$10], name2=[$11])
     LogicalJoin(condition=[=($6, $10)], joinType=[inner])
       LogicalJoin(condition=[=($0, $6)], joinType=[inner])
         LogicalJoin(condition=[=($0, $4)], joinType=[inner])
           LogicalTableScan(table=[[depts]])
           LogicalTableScan(table=[[emps]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
   ```
   
   And the whole best plan after this rule is :
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalJoin(condition=[=($3, $0)], joinType=[inner])
         LogicalTableScan(table=[[depts]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   
   You can see that 'depts' now become a self-join after rule, and join-key of this self-join is unique key , then we can remove this join further, like:
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   It doesn't mean to just simply remove the join , but instead with it child relNode , because they are equivalent.



##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   The initial plan is :
   ```
   LogicalProject(deptno=[$0], added=[$1], name=[$2], empid=[$3], deptno0=[$4], name0=[$5], deptno1=[$6], added0=[$7], name1=[$8], empid0=[$9], deptno2=[$10], name2=[$11])
     LogicalJoin(condition=[=($6, $10)], joinType=[inner])
       LogicalJoin(condition=[=($0, $6)], joinType=[inner])
         LogicalJoin(condition=[=($0, $4)], joinType=[inner])
           LogicalTableScan(table=[[depts]])
           LogicalTableScan(table=[[emps]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
   ```
   
   And the whole best plan after this rule is :
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalJoin(condition=[=($3, $0)], joinType=[inner])
         LogicalTableScan(table=[[depts]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   
   You can see that 'depts' now become a self-join after rule, and join-key of this self-join is unique key , then we can remove this join further, like:
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   It doesn't mean to just simply remove the join , but instead with it child relNode , because they are equivalent.



##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""
+        + "LogicalJoin(condition=[=($3, $0)], joinType=[inner])\n"

Review Comment:
   Actually it doesn't remove self-join now in LoptOptimizeJoinRule, it just reorder the join factors to find removable self-join and put them together. After that , we can remove this self-join further in our custom optimize rule. My response to your next commit may help to understand.
   And thanks for your review!



-- 
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


[GitHub] [calcite] LakeShen commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   What does the initial plan look like?



##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   What does the initial plan look like?



-- 
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


[GitHub] [calcite] LakeShen commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java:
##########
@@ -2071,12 +2071,12 @@ private static boolean areSelfJoinKeysUnique(RelMetadataQuery mq,
     for (IntPair pair : joinInfo.pairs()) {
       final RelColumnOrigin leftOrigin =
           mq.getColumnOrigin(leftRel, pair.source);
-      if (leftOrigin == null || !leftOrigin.isDerived()) {

Review Comment:
   Could you give some code comments explaining why you changed this code?
   
   Code comments could help people better to understand why you made a change.



##########
core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java:
##########
@@ -2071,12 +2071,12 @@ private static boolean areSelfJoinKeysUnique(RelMetadataQuery mq,
     for (IntPair pair : joinInfo.pairs()) {
       final RelColumnOrigin leftOrigin =
           mq.getColumnOrigin(leftRel, pair.source);
-      if (leftOrigin == null || !leftOrigin.isDerived()) {
+      if (leftOrigin == null || leftOrigin.isDerived()) {
         return false;
       }
       final RelColumnOrigin rightOrigin =
           mq.getColumnOrigin(rightRel, pair.target);
-      if (rightOrigin == null || !rightOrigin.isDerived()) {
+      if (rightOrigin == null || rightOrigin.isDerived()) {

Review Comment:
   Same as above.



-- 
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


[GitHub] [calcite] wq352400523 commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java:
##########
@@ -2071,12 +2071,12 @@ private static boolean areSelfJoinKeysUnique(RelMetadataQuery mq,
     for (IntPair pair : joinInfo.pairs()) {
       final RelColumnOrigin leftOrigin =
           mq.getColumnOrigin(leftRel, pair.source);
-      if (leftOrigin == null || !leftOrigin.isDerived()) {

Review Comment:
   Update the code, thanks



-- 
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


[GitHub] [calcite] LakeShen commented on pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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

   Hi @wq352400523 ,you should rebase the  latest code of main branch,after that,your pr's commits and file changed would be clean.


-- 
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


[GitHub] [calcite] wq352400523 commented on a diff in pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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


##########
core/src/test/java/org/apache/calcite/tools/PlannerTest.java:
##########
@@ -1576,4 +1583,77 @@ private static class VaryingTypeSystem extends DelegatingTypeSystem {
       return true;
     }
   }
+
+  /**
+   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5927">[CALCITE-5927]
+   * Reorder join condition if it contains self-join</a>.
+   */
+  @Test public void testReorderForSelfJoin() throws Exception {
+    final String sql = "select * from \"depts\" as d0\n"
+        + "join \"emps\" as d1 on d0.\"deptno\" = d1.\"deptno\"\n"
+        + "join \"depts\" as d2 on d0.\"deptno\" = d2.\"deptno\"\n"
+        + "join \"emps\" as d3 on d2.\"deptno\" = d3.\"deptno\"\n";
+
+    SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    // we add customized table into root schema instead of using HrSchema,
+    // because we need statistics
+    rootSchema.add("depts", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("added", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(245D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+    rootSchema.add("emps", new AbstractTable() {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("empid", SqlTypeName.INTEGER)
+            .add("deptno", SqlTypeName.INTEGER)
+            .add("name", SqlTypeName.VARCHAR)
+            .build();
+      }
+      @Override public Statistic getStatistic() {
+        return Statistics.of(240D,
+            ImmutableList.of(ImmutableBitSet.of(0)));
+      }
+    });
+
+    final FrameworkConfig config = Frameworks.newConfigBuilder()
+        .parserConfig(SqlParser.Config.DEFAULT.withCaseSensitive(false))
+        .defaultSchema(rootSchema)
+        .ruleSets()
+        .build();
+
+    final Planner planner = Frameworks.getPlanner(config);
+    SqlNode sqlNode = planner.parse(sql);
+    sqlNode = planner.validate(sqlNode);
+    RelNode relNode = planner.rel(sqlNode).rel;
+
+    final HepProgram program =
+        HepProgram.builder()
+            .addMatchOrder(HepMatchOrder.BOTTOM_UP)
+            .addGroupBegin()
+            .addRuleInstance(CoreRules.PROJECT_REMOVE)
+            .addRuleInstance(CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE)
+            .addRuleInstance(CoreRules.PROJECT_MERGE)
+            .addGroupEnd()
+            .addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN)
+            .addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE)
+            .build();
+
+    final HepPlanner hepPlanner = new HepPlanner(program);
+    hepPlanner.setRoot(relNode);
+    RelNode bestNode = hepPlanner.findBestExp();
+    final String expected = ""

Review Comment:
   The initial plan is :
   ```
   LogicalProject(deptno=[$0], added=[$1], name=[$2], empid=[$3], deptno0=[$4], name0=[$5], deptno1=[$6], added0=[$7], name1=[$8], empid0=[$9], deptno2=[$10], name2=[$11])
     LogicalJoin(condition=[=($6, $10)], joinType=[inner])
       LogicalJoin(condition=[=($0, $6)], joinType=[inner])
         LogicalJoin(condition=[=($0, $4)], joinType=[inner])
           LogicalTableScan(table=[[depts]])
           LogicalTableScan(table=[[emps]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
   ```
   
   And the whole best plan after this rule is :
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalJoin(condition=[=($3, $0)], joinType=[inner])
         LogicalTableScan(table=[[depts]])
         LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   
   You can see that 'depts' now become a self-join after rule, and join-key of this self-join is unique key , then we can remove this join further, like:
   ```
   LogicalProject(deptno=[$3], added=[$4], name=[$5], empid=[$9], deptno0=[$10], name0=[$11], deptno1=[$0], added0=[$1], name1=[$2], empid0=[$6], deptno2=[$7], name2=[$8])
   LogicalJoin(condition=[=($3, $10)], joinType=[inner])
     LogicalJoin(condition=[=($0, $7)], joinType=[inner])
       LogicalTableScan(table=[[depts]])
       LogicalTableScan(table=[[emps]])
     LogicalTableScan(table=[[emps]])
   ```
   It doesn't mean to just simply remove the join , but instead with it child relNode , because they are equivalent.



-- 
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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3379)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL)
   
   [![60.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '60.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list) [60.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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-5927] LoptOptimizeJoinRule miss removable self-join when col… [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu closed pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…
URL: https://github.com/apache/calcite/pull/3379


-- 
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-5927] LoptOptimizeJoinRule miss removable self-join when col… [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


-- 
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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3379)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3379&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3379&resolved=false&types=CODE_SMELL)
   
   [![60.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '60.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list) [60.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3379&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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-5927] LoptOptimizeJoinRule miss removable self-join when col… [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


-- 
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