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 support for SQLite shorthand form in foreign key constraints #6010

Merged
merged 1 commit into from
May 8, 2023

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Apr 14, 2023

Q A
Type feature
Fixed issues #5941

Summary

Introspect the referenced table in order to get the foreign column.

Closes #5941.

ToDo

  • Check support for composite columns.

@phansys phansys marked this pull request as ready for review April 14, 2023 01:18
@greg0ire
Copy link
Member

What if the primary key of the foreign table is composite? What should happen then?

@phansys
Copy link
Contributor Author

phansys commented Apr 14, 2023

I didn't know SQLite was supporting single column and composite (multiple column) primary keys.
To keep it simple, my first though would be to add support only for single column keys in this PR and throw an exception if it is composite, allowing the further development to implement the remaining part.
But let me confirm if I can give it a try tomorrow before giving up so quickly 😄

@derrabus
Copy link
Member

As far as I understand, DBAL would not emit the kind of shorthand syntax that you use in that functional test of yours. It would instead emit a more verbose version that explicitly references the foreign column. So the problem you're solving here applies to databases that have not been created by DBAL.

My question: If I encounter such a table, would DBAL try to migrate it to the more verbose version or leave it as it is? Can we cover the answer to that question with a test? 🙃

@phansys phansys changed the base branch from 3.6.x to 3.7.x April 14, 2023 09:35
@phansys phansys changed the title Add missing support for SQLite shorthand form in foreign key constraints Add support for SQLite shorthand form in foreign key constraints Apr 14, 2023
@phansys phansys force-pushed the issue_5941 branch 5 times, most recently from 460d06c to f79a428 Compare April 14, 2023 10:56
@phansys phansys marked this pull request as ready for review April 14, 2023 11:19
@derrabus derrabus added this to the 3.7.0 milestone May 8, 2023
@derrabus derrabus merged commit fd47abd into doctrine:3.7.x May 8, 2023
71 checks passed
@phansys phansys deleted the issue_5941 branch May 8, 2023 10:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not support shorthand form to create the foreign key constraint
3 participants