You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "mihaibudiu (via GitHub)" <gi...@apache.org> on 2025/09/23 06:23:27 UTC
[PR] [CALCITE-7191] Hypergraph creation with incorrect hyperedges [calcite]
mihaibudiu opened a new pull request, #4552:
URL: https://github.com/apache/calcite/pull/4552
This fix is described in detail in the JIRA ticket, and it was created by Nirmal Govindaraj
I have only expedited the implementation.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326389766
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552)
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3325685476
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552)
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3329994420
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552)
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "silundong (via GitHub)" <gi...@apache.org>.
silundong commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326547298
If you're just looking for an example where the result isn't a chain, try changing the row count of all tables to 10. You'll get the following plan.
```
LogicalJoin(condition=[=($26, $9)], joinType=[inner])
LogicalJoin(condition=[=($13, $2)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, SALGRADE]])
LogicalJoin(condition=[=($0, $5)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, EMP_ADDRESS]])
LogicalJoin(condition=[=($9, $0)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalJoin(condition=[=($9, $1)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalTableScan(table=[[CATALOG, SALES, BONUS]])
```
Enabling dphyp logging reveals that this plan indeed has the lowest cost.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3322646238
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4552&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4552&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4552&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4552)
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "gnirmal1 (via GitHub)" <gi...@apache.org>.
gnirmal1 commented on code in PR #4552:
URL: https://github.com/apache/calcite/pull/4552#discussion_r2373931747
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -277,6 +277,55 @@ private static boolean skipItem(RexNode expr) {
.check();
}
+ private HepProgram createHypergraphProgram() {
+ return new HepProgramBuilder().addMatchOrder(HepMatchOrder.BOTTOM_UP)
+ .addRuleInstance(CoreRules.JOIN_PROJECT_RIGHT_TRANSPOSE)
+ .addRuleInstance(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE)
+ .addRuleInstance(CoreRules.PROJECT_MERGE)
+ .addRuleInstance(CoreRules.PROJECT_REMOVE)
+ .addRuleInstance(CoreRules.JOIN_TO_HYPER_GRAPH)
+ .build();
+ }
+
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7191">[CALCITE-7191]
+ * Hypergraph creation with incorrect hyperedges</a>. */
+ @Test void testHypergraph0() {
+ HepProgram program = createHypergraphProgram();
+ String innerJoinSql = "select a.ename, bc.name from bonus a inner join "
+ + "(select b.empno, b.ename, c.name from emp b inner join dept c on b.deptno = c.deptno) bc "
+ + "on a.ename = bc.ename";
+
+ sql(innerJoinSql).withPre(program)
+ .withRule(CoreRules.HYPER_GRAPH_OPTIMIZE, CoreRules.PROJECT_MERGE)
+ .check();
+ }
+
Review Comment:
The same query has been duplicated for testHypergraph1 and testHypergraph2
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "gnirmal1 (via GitHub)" <gi...@apache.org>.
gnirmal1 commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326346999
I don't have a testcase that would produce a balanced tree right now, would need to study the costs being passed by the mock schemas to construct one
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326344350
I can't imagine that the optimal join order is always a list. Do you have an example where a tree is produced?
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3329146423
Thanks @silundong
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4552:
URL: https://github.com/apache/calcite/pull/4552#discussion_r2373936233
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -277,6 +277,55 @@ private static boolean skipItem(RexNode expr) {
.check();
}
+ private HepProgram createHypergraphProgram() {
+ return new HepProgramBuilder().addMatchOrder(HepMatchOrder.BOTTOM_UP)
+ .addRuleInstance(CoreRules.JOIN_PROJECT_RIGHT_TRANSPOSE)
+ .addRuleInstance(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE)
+ .addRuleInstance(CoreRules.PROJECT_MERGE)
+ .addRuleInstance(CoreRules.PROJECT_REMOVE)
+ .addRuleInstance(CoreRules.JOIN_TO_HYPER_GRAPH)
+ .build();
+ }
+
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7191">[CALCITE-7191]
+ * Hypergraph creation with incorrect hyperedges</a>. */
+ @Test void testHypergraph0() {
+ HepProgram program = createHypergraphProgram();
+ String innerJoinSql = "select a.ename, bc.name from bonus a inner join "
+ + "(select b.empno, b.ename, c.name from emp b inner join dept c on b.deptno = c.deptno) bc "
+ + "on a.ename = bc.ename";
+
+ sql(innerJoinSql).withPre(program)
+ .withRule(CoreRules.HYPER_GRAPH_OPTIMIZE, CoreRules.PROJECT_MERGE)
+ .check();
+ }
+
Review Comment:
that's careless.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "gnirmal1 (via GitHub)" <gi...@apache.org>.
gnirmal1 commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3322789624
One suggestion, we can add 2 more testcases that hits the flow
```
HepProgram program = new HepProgramBuilder()
.addMatchOrder(HepMatchOrder.BOTTOM_UP)
.addRuleInstance(CoreRules.JOIN_PROJECT_RIGHT_TRANSPOSE)
.addRuleInstance(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE)
.addRuleInstance(CoreRules.PROJECT_MERGE)
.addRuleInstance(CoreRules.PROJECT_REMOVE)
.addRuleInstance(CoreRules.JOIN_TO_HYPER_GRAPH)
.build();
String innerJoinSql = "select a.ename, bcde.name " +
"from bonus a " +
"inner join (" +
" select b.empno, b.ename, c.name " +
" from emp b " +
" inner join dept c on b.deptno = c.deptno " +
" inner join emp_address d on d.empno = b.empno " +
" inner join salgrade e on b.sal = e.hisal" +
") bcde on a.ename = bcde.ename";
sql(innerJoinSql)
.withPre(program)
.withRule(CoreRules.HYPER_GRAPH_OPTIMIZE, CoreRules.PROJECT_MERGE)
.check();
```
```
HepProgram program = new HepProgramBuilder()
.addMatchOrder(HepMatchOrder.BOTTOM_UP)
.addRuleInstance(CoreRules.JOIN_PROJECT_RIGHT_TRANSPOSE)
.addRuleInstance(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE)
.addRuleInstance(CoreRules.PROJECT_MERGE)
.addRuleInstance(CoreRules.PROJECT_REMOVE)
.addRuleInstance(CoreRules.JOIN_TO_HYPER_GRAPH)
.build();
String innerJoinSql = "select ab.ename, cdef.name " +
"from (select a.ename from bonus a inner join emp b on a.ename = b.ename) ab " +
"inner join (" +
" select c.empno, c.ename, d.name " +
" from emp c " +
" inner join dept d on c.deptno = d.deptno " +
" inner join emp_address e on e.empno = c.empno " +
" inner join salgrade f on c.sal = f.hisal" +
") cdef on ab.ename = cdef.ename";
sql(innerJoinSql)
.withPre(program)
.withRule(CoreRules.HYPER_GRAPH_OPTIMIZE, CoreRules.PROJECT_MERGE)
.check();
```
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "gnirmal1 (via GitHub)" <gi...@apache.org>.
gnirmal1 commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326344295
Creating a balanced tree of joins would be tricky as that would also depend on the cost model. These testcases atleast hit the multiple cases of the JoinToHypergraph flow
Changes look good, LGTM !
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326303215
I have fixed the second commit. Sorry for the needless round trips.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326344734
I will merge this once CI passes.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3329921482
I have squashed the commits so I can merge this.
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #4552:
URL: https://github.com/apache/calcite/pull/4552
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3325103927
@gnirmal1 I have added the two tests in a separate commit.
If you approve I plan to merge this soon.
I haven't validated manually the hypergraph produced by these tests...
--
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-7191] Hypergraph creation with incorrect hyperedges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4552:
URL: https://github.com/apache/calcite/pull/4552#issuecomment-3326306803
incidentally, notice that both plans are lists of joins; I never managed to convince the hypergraph algorithm to produce a balanced tree of 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