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

Is a migration that renames a primary key on a parent table supposed to account for child table foreign keys? #25040

Closed
AlilNinja opened this issue Jun 5, 2021 · 5 comments

Comments

@AlilNinja
Copy link

AlilNinja commented Jun 5, 2021

When moving from EF Core 3.1.7 to EF Core 5.0.5 the naming conventions around pluralization of primary keys/indexes changed slightly, so we needed to generate a migration to update our names. One primary key in particular is causing a problem because there's another table that has a foreign key that references it. The scenario in pseudocode is pretty basic (should be anything where a parent table has a primary key and there's a child table that has a foreign key to the parent primary key - though in my case it's a one-to-many relationship):

public class Foo : IKeyedEntity
{
	public Guid Id //parent table primary key
	public string FooName
	public int FooNumber
	public IReadOnlyCollection<Bar> Bars //one-to-many child table
}

public class Bar : IKeyedEntity
{
	public Guid Id
	public enum BarType
	public Guid FooId //child table foreign key on the parent table's primary key
}

The migration generates fine, but only includes the rename for the primary key. Because of that, when we run the migration and the default "renaming" of dropping the key and then re-adding it happens, the migration errors out saying it can't drop the constraint because other objects depend on it. This is because the migration script isn't taking into account that it needs to drop the child table's foreign key before the parent table primary key can be dropped. Right now I have two solutions/workarounds:

  • manually edit the migration so that it drops the foreign key on the child table first, then does the drop and re-add of the parent table primary key, then add the foreign key back
  • Add a .HasName to the Primary key in question to lock in the name so it doesn't have to change

My question is: is this how it's supposed to work? Is it expected that EF Core doesn't pick up foreign keys on a primary key name change and you have to manually edit the migration to fix it? Or is there some sort of configuration I'm missing and I could tell entity framework to generate the drop and re-add for the child table foreign key as well? (basically manual fix option 1 but automatically)

Other info:
The closest thing I could find to my problem was issue #15084 except they're talking about renaming a column rather than a key on an existing column. Issue #13178 that was linked about foreign keys is also about changing a column's property rather than a key
EF Core version: 3.1.7, 5.0.5 (what we're migrating from and to, but the same migration is generated in both)
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL
Target framework: .NET 5.0

@roji
Copy link
Member

roji commented Jun 8, 2021

Support for renaming primary/foreign keys (without dropping/rebuilding) is tracked by #5662 (#14988 was submitted at some point).

@AlilNinja
Copy link
Author

Ah, I had looked at #5662 but stopped after the initial question cuz I wasn't looking to add a rename operation. I now see a bit further down Brar makes a comment that is exactly my problem. It looks like his PR was ultimately abandoned and closed, but either way it's not quite the issue I'm asking about.

I don't mind whether a name change for a primary key constraint is a drop and a recreate, or some special rename operation. My issue is that when a primary key constraint has its name changed, if that constraint is referenced by a foreign key constraint then the migration that's generated fails due to a constraint error. This is cuz it only drops and recreates the primary key and doesn't account for any foreign keys. That feels like something entity framework should be able to do. Basically, if it's gonna drop and re-create a primary key constraint, it should drop the foreign key constraints that reference it first and then recreate them after it recreates the primary key. Is there a way right now to get it to do that? (some sort of configuration or convention?) Or is a manual intervention in the migration file required to fix the issue?

@roji
Copy link
Member

roji commented Jun 8, 2021

@AlilNinja the point is that if we rename a primary key instead of dropping and re-creating it, nothing would need to be done to the foreign keys which point to it - the problem is specifically with the dropping of a referenced foreign key. Now, there may be some other scenarios which could require dropping and recreating a primary key (e.g. changing its type or some other facet, such as SQL Server identity), but any scenario which can be handled via renaming should probably be done that way.

Or is a manual intervention in the migration file required to fix the issue?

For now, the easiest workaround would probably be to edit your migration files after they are scaffolded, and either:

  1. Replace the drop/recreate with raw SQL which does a rename (in PG: ALTER TABLE name RENAME CONSTRAINT constraint_name TO new_constraint_name).
  2. Remove the drop/recreate altogether, which will leave your primary key constraint alone. Its name would no longer be consistent with the table, but that has little real consequences.

@AlilNinja
Copy link
Author

Oh! I get it now. For some reason I thought all the talk of dropping+recreating being the norm meant that a rename operation for primary keys didn't exist. My bad XD

I went with the raw SQL option since it takes care of the problem and is super straightforward, and it worked splendidly. Thanks!

TLDR for anyone looking at this in the future: as of the time of this issue, entity framework doesn't automatically account for it and you must fix it manually, but a raw SQL rename is an easy fix

@roji
Copy link
Member

roji commented Jun 8, 2021

Duplicate of #5662

@roji roji marked this as a duplicate of #5662 Jun 8, 2021
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants