[CALCITE-5756] Expand ProjectJoinRemoveRule to support inner join remove by the foreign-unique constraint in the catalog

[JingDas]

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:

1. Analyse the column used by project, and then split them to left and right side.
2. Acccording to the project info above and outer join type, bail out in some scene.
3. Get join info such as join keys.
4. For inner join check foreign and unique keys, these may use
   RelMetadataQuery#getForeignKeys(newly add, similar to RelMetadataQuery#getUniqueKeys),
   RelOptTable#getReferentialConstraints.
5. Check removing side join keys are areColumnsUnique both for outer join and inner join.
6. If all done, calculate the fianl project and transform.

Test cases are added to RelOptRulesTest and RelMetadataTest.

[asolimando]

LGTM, lots of minor comments and some missing tests but the PR is in good shape and the contribution is very valuable, will get there in few iterations, thanks @JingDas for your work!

[asolimando]

Almost forgot: the commit message must match the title of the Jira ticket, at the moment they are different

[asolimando]

@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

[asolimando]

I have reviewed your changes, I have some further comments but we are converging.

I'd ask you the favour of not resolving comments on your side, as it makes it very tricky for me to track if what was asked has been addressed or not, I will mark them as resolved myself, thanks!

[asolimando]

LGTM, can you squash your changes and make sure the only commit has [CALCITE-5756] Expand ProjectJoinRemoveRule to support inner join removal by using the foreign-unique constraints matching the commit message like the associated Jira ticket?

I will merge in the next 24 hours if no objections arise.

As a side note, avoid rebasing (or squashing) until the very end of the review as it makes it harder to check changes incrementally.

[asolimando]

Adding "request review" from another committer given the comments in the associated Jira ticket.

[JingDas]

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?

[asolimando]

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?

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.

[JingDas]

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?

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.

[JingDas]

Sorry, it was closed by mistake，I reopen it and fixed the most necessary SonarCloud detected code.

[JingDas]

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.

[JingDas]

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.

[JingDas]

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.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

83.8% Coverage
0.0% Duplication

[mihaibudiu]

This PR was approved, but never merged. What's the plan?

[asolimando]

@mihaibudiu, discussion in the associated Jira ticket led to creating CALCITE-5881, which needs to be addressed before getting this in, unfortunately there was no progress on it for a long time now. I will remove the approval to make the situation clearer, thanks for the reminder.