-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[FLINK-22113][table-planner-blink] Implement the column uniqueness checking for TableSourceTable #15744
Conversation
…ecking for TableSourceTable
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit ce101bd (Sun Apr 25 03:40:03 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
@xuguangheng Thank you for your effort!
|
Hi @xuguangheng, apologies for the delay in looking at your PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @xuguangheng , and so sorry for the late response
if (schema.getPrimaryKey.isPresent) { | ||
val columns = schema.getFieldNames | ||
val columnIndices = schema.getPrimaryKey.get().getColumns map { c => | ||
columns.indexOf(c) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic is wrong when parts of primary keys are projected. please refer to FlinkRelMdUniqueKeys#getTableUniqueKeys(RelOptTable) to correct the logic
Is there any progress here? @xuguangheng |
@xuguangheng Again, apologies for the delay, but since we'd like to get forward and fix this soon, I'd like to take over, I hope that's fine for you. |
Thanks for taking over. |
close it with: superseded by #17962 |
What is the purpose of the change
Implement the column uniqueness checking to avoid losing uniqueness information during join operations.
More specifically,
VS
The only difference is that we select booking_channel_key from both left and right table in the first case and only booking_channel_key from left table in the second case. The result is one has uniqueKey and the other doesn't.
t.booking_channel_key, t.passenger_key
can be the unique key of the first join output, but it seems we lost this unique information.Brief change log
Verifying this change
Existing and added test cases, manually rerun the example above and the uniqueness information was remained successfully.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation