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

Add ForeignKey constraint type #8566

Closed
wants to merge 6 commits into from

Conversation

simonvandel
Copy link
Contributor

@simonvandel simonvandel commented Dec 16, 2023

Which issue does this PR close?

Closes #.

Rationale for this change

While toying around with Datafusion, I would like to use foreign key relationship metadata.

What changes are included in this PR?

Extended Constraint with ForeignKey.
Added conversion from sqslparser's TableConstraint.

Are these changes tested?

Yes, added a test in sql_integration.rs

Are there any user-facing changes?

Yes.

It's now possible to mark columns as foreign keys to other tables.

Adding a new enum variant is a breaking change.

@alamb
Copy link
Contributor

alamb commented Dec 17, 2023

Thank you @simonvandel -- the basic idea looks good to me. It seems we have a few CI failures on this PR

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jan 5, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @simonvandel -- this is looking pretty close. I think there is a bug here, and it would be nice to see some tests that showed that the index lookups are working correctly

datafusion/common/src/functional_dependencies.rs Outdated Show resolved Hide resolved
Copy link

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale PR has not had any activity for some time label Apr 14, 2024
@andygrove
Copy link
Member

Hi @simonvandel. Do you still plan on working on this?

@simonvandel
Copy link
Contributor Author

Hi @andygrove
I don't have a use case anymore, so I'll just close the PR.

If someone else wants the changes, they can use my changes in a new PR.

@alamb
Copy link
Contributor

alamb commented Apr 19, 2024

Thanks @simonvandel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner sqllogictest SQL Logic Tests (.slt) Stale PR has not had any activity for some time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants