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

update foreign keys when table name changes #5482

Merged
merged 6 commits into from
Feb 14, 2020

Conversation

osdiab
Copy link
Contributor

@osdiab osdiab commented Feb 5, 2020

Closes #5119 , based on #5120

Includes a check for the correct table name when checking for equality of foreign keys, to decide if they should be dropped and made fresh

@osdiab
Copy link
Contributor Author

osdiab commented Feb 7, 2020

Not sure who to tap for review - @pleerock how do I decide?

@osdiab
Copy link
Contributor Author

osdiab commented Feb 7, 2020

Cc @csuich2

@osdiab
Copy link
Contributor Author

osdiab commented Feb 14, 2020

ping @pleerock @Kononnable @AlexMesser not sure who is maintaining this project

@pleerock
Copy link
Member

Schema synchronization is the part developed by @AlexMesser and I'm really scary to touch these areas. Hope @AlexMesser can review this PR.

`ALTER TABLE "post" DROP CONSTRAINT "FK_4490d00e1925ca046a1f52ddf04"`
]);
} finally {
connection.close();
Copy link
Member

Choose a reason for hiding this comment

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

we need to await it here

@AlexMesser
Copy link
Collaborator

This is an acceptable solution for now, but later we need to include referenced table name in to foreign key name to keep synchronization process consistent

@csuich2
Copy link

csuich2 commented Feb 14, 2020

I had the same thought about including table name in the foreign key name, but that would cause all existing foreign key hashes to change, so a fairly significant, if not breaking, change.

Should we consider that for the next major release?

@pleerock
Copy link
Member

@csuich2 maybe not next, but next next next one :D Let's merge it for now. Thanks!

@pleerock pleerock merged commit 7157cb3 into typeorm:master Feb 14, 2020
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.

Foreign keys whose referenced table change are not updated by migrations
4 participants