Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-5939] Support mv rewrite between join types #3381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

suibianwanwank
Copy link

No description provided.

import static org.apache.calcite.plan.expand.SubstitutionRuleUtils.tryMergeParentCalcAndGenResult;

/**
* Enhancements to joinOnLeftCalcToJoinUnifyRule.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the comments more detailed? It's better to include examples in the comments so that others can understand them easily,the other classes are the same

@LakeShen
Copy link
Contributor

Please check you PR workflow,there are many failures.

/**
* Enhancements to joinOnRightCalcToJoinUnifyRule.
*/
public class ExpandJoinOnRightCalcToJoinUnifyRule
Copy link
Contributor

Choose a reason for hiding this comment

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

ExpandJoinOnRightCalcToJoinUnifyRule,ExpandJoinOnLeftCalcToJoinUnifyRule,ExpandJoinOnCalcToJoinUnifyRule,What is the difference between them?

Copy link
Author

Choose a reason for hiding this comment

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

You can see that there are prototypes of these three rules in the rules that already exist in SubstitutionVisitor, because the unifyRule is an adaptation of the operator to the operator level, so different input structures of Join are treated differently, which you can also reflect in tests
For example, the following two SQL statements correspond to JoinOnLeftCalc and JoinOnRightCalc, respectively
select A.empid, A.deptno, depts.deptno from (select empid, deptno from emps where deptno > 10) A join depts on A.deptno = depts.deptno ;
select A.empid, A.deptno, depts.deptno from depts join (select empid, deptno from emps where deptno > 10) A on depts.deptno = A.deptno ;

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your attention to this issue, after some time to verify, I refactored this part of the code, optimized some processes and added more comments, you can reread this code if you're interested @LakeShen

@suibianwanwank
Copy link
Author

Please check you PR workflow,there are many failures.

ok,I will fix it in these days

@sonarcloud
Copy link

sonarcloud bot commented Aug 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 19 Code Smells

86.5% 86.5% Coverage
7.9% 7.9% Duplication

Copy link

sonarcloud bot commented Feb 27, 2024

@suibianwanwank suibianwanwank changed the title [CALCITE-5939] Support for rewriting different join types in UnifyRule [CALCITE-5939] Support mv rewrite between join types Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants