[CALCITE-5756] Expand ProjectJoinRemoveRule to support inner join remove by the foreign-unique constraint in the catalog
This feature is mainly about expanding ProjectJoinRemoveRule to support inner join remove by the foreign-unique constraint in the catalog.
The main steps are as follows:
- Analyse the column used by project, and then split them to left and right side.
- Acccording to the project info above and outer join type, bail out in some scene.
- Get join info such as join keys.
- For inner join check foreign and unique keys, these may use RelMetadataQuery#getForeignKeys(newly add, similar to RelMetadataQuery#getUniqueKeys), RelOptTable#getReferentialConstraints.
- Check removing side join keys are areColumnsUnique both for outer join and inner join.
- If all done, calculate the fianl project and transform.
Test cases are added to RelOptRulesTest and RelMetadataTest.
Almost forgot: the commit message must match the title of the Jira ticket, at the moment they are different
@JingDas thanks for addressing the comments, I am on a business trip at the moment, I should be able to review again by the end of this week.
FYI: when you are done with comments and you want the reviewers to take another look you can also use the "request review" button close to the reviewers' name in the top-right part of the GitHub page
Adding "request review" from another committer given the comments in the associated Jira ticket.
The RelOptForeignKey component has been added to represent the composite or single foreign key and the unique constraint that foreign key references.
Sorry to bother you, @asolimando @julianhyde would you help me to review the code?
The
RelOptForeignKeycomponent has been added to represent the composite or single foreign key and the unique constraint that foreign key references. Sorry to bother you, @asolimando @julianhyde would you help me to review the code?
Thanks @JingDas, I will review again as soon as I have some time.
In the meantime, I have approved the pending workflows, but I have also noticed that there are 18 code smells detected by SonarCloud, could you do a pass on this list and fix whatever can be reasonably fixed?
As for any automatic tool not everything makes sense and there might even be false positives.
The
RelOptForeignKeycomponent has been added to represent the composite or single foreign key and the unique constraint that foreign key references. Sorry to bother you, @asolimando @julianhyde would you help me to review the code?Thanks @JingDas, I will review again as soon as I have some time.
In the meantime, I have approved the pending workflows, but I have also noticed that there are 18 code smells detected by SonarCloud, could you do a pass on this list and fix whatever can be reasonably fixed?
As for any automatic tool not everything makes sense and there might even be false positives.
Sorry, it was closed by mistake,I reopen it and fixed the most necessary SonarCloud detected code.
I some confusion about the failed results of the CI tests. CI failed test log as fllowing:
org.apache.calcite.test.RelOptRulesTest > testProjectJoinRemove25() failure marker
FAILURE 0.0sec, org.apache.calcite.test.RelOptRulesTest > testProjectJoinRemove25()
java.lang.AssertionError: Expected planAfter must not be present when using unchanged=true or calling checkUnchanged.
at org.apache.calcite.test.RelOptFixture.checkPlanning(RelOptFixture.java:397)
at org.apache.calcite.test.RelOptFixture.check(RelOptFixture.java:330)
at org.apache.calcite.test.RelOptFixture.checkUnchanged(RelOptFixture.java:322)
at org.apache.calcite.test.RelOptRulesTest.testProjectJoinRemove25(RelOptRulesTest.java:6297)
In RelOptFixture.java:397 it throw the exception.
But in RelOptRulesTest#testProjectJoinRemove25, I use checkUnchanged() method which means unchanged is true. It should run into RelOptFixture.java:395 instead of RelOptFixture.java:397.
Aha! It seems calcite main code change, And the CI seems to use the new main logic. I will fix it according to main code usage.
Aha! It seems calcite main code change, And the CI seems to use the new main logic. I will fix it according to main code usage.
Have fixed it.
This PR was approved, but never merged. What's the plan?
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 [email protected] list. Thank you for your contributions.
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 [email protected] list. Thank you for your contributions.
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 [email protected] list. Thank you for your contributions.
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 [email protected] list. Thank you for your contributions.







