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

fix(create_table): avoid creating unintended foreign keys #1130

Merged
merged 4 commits into from
Feb 17, 2025

Conversation

bevzzz
Copy link
Collaborator

@bevzzz bevzzz commented Feb 15, 2025

Foreign keys should be created for has-one and belongs-to relations iff:

  • None of the referencing columns is a primary keys
  • The table is an m2m 'junction' table an all referencing columns are primary keys

See more details in this comment.

Closes #1115

Foreign keys should only be created for has-one and belongs-to relations iff:
- None of the referencing columns is a primary keys
- The table is an m2m 'junction' table an all referencing columns are primary keys
The m2m edge case is covered by TestDatabaseInspector_Inspect
@bevzzz bevzzz requested a review from vmihailenco February 16, 2025 00:00
@j2gg0s
Copy link
Collaborator

j2gg0s commented Feb 17, 2025

LGTM

@bevzzz Can you help resolve the conflict caused by a previous merge?

@bevzzz bevzzz requested a review from j2gg0s February 17, 2025 11:44
@bevzzz
Copy link
Collaborator Author

bevzzz commented Feb 17, 2025

Can you help resolve the conflict caused by a previous merge?

Done 👌
Awaiting your approval @j2gg0s.

Btw, do we squash PR commits or not?

@j2gg0s
Copy link
Collaborator

j2gg0s commented Feb 17, 2025

Btw, do we squash PR commits or not?

I personally prefer squash commits, so that the master branch focuses only on the final result rather than the intermediate process.

However, this is not currently mandatory.

@bevzzz
Copy link
Collaborator Author

bevzzz commented Feb 17, 2025

LGTM

I'm going to take that as a green light and merge this, @j2gg0s

@bevzzz bevzzz merged commit 187743b into master Feb 17, 2025
5 checks passed
@bevzzz bevzzz deleted the fix/has-one-unintended-fk branch February 17, 2025 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rel:has-one on a relationship should not have foreign key creation
2 participants