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

Migrations are too aggressive on RenameColumn #15178

Open
Tracked by #22946
yepeekai opened this issue Mar 27, 2019 · 9 comments
Open
Tracked by #22946

Migrations are too aggressive on RenameColumn #15178

yepeekai opened this issue Mar 27, 2019 · 9 comments

Comments

@yepeekai
Copy link

We made huge refactoring of our model for a new application. There is a lot of RenameColumn in the generated Migration and a lot of them don't make sense: Foreign keys have been removed and added to different tables and EF Core choose to rename columns even when the foreign key is for a different table.

If we drop EntityAId (pointing to EntityA) and add EntityBId (pointing to EntityB), it shouldn't be a rename but a drop and add.

I think it should be a DropColumn and a AddColumn instead of a RenameColumn for a foreign key that points to different entities.

@ajcvickers
Copy link
Contributor

@yepeekai It's not possible to know for sure when something is a rename, so sometimes the heuristics will detect dropping and adding as a rename when it isn't. We discussed this in triage and didn't come up with anything specific we can do better, but if you have any suggestions from your specific case we would be interested in hearing them.

@yepeekai
Copy link
Author

I think the example I gave would qualify as a suggestion on a case that should not be a rename... If it is a foreign key and it is for a different table then it is not a rename...

@ajcvickers ajcvickers reopened this Mar 29, 2019
@ajcvickers
Copy link
Contributor

@yepeekai Thanks--we will discuss.

@ajcvickers ajcvickers added this to the Backlog milestone Apr 5, 2019
@ajcvickers ajcvickers added type-enhancement help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. customer-reported and removed closed-by-design labels Apr 5, 2019
@divega divega added good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. good first issue This issue should be relatively straightforward to fix. labels May 31, 2019
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label May 31, 2019
@ajcvickers ajcvickers removed the help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. label Aug 5, 2019
@bricelam bricelam self-assigned this Aug 13, 2019
@bricelam bricelam modified the milestones: Backlog, 5.0.0 Nov 13, 2019
Muppets added a commit to Muppets/EntityFrameworkCore that referenced this issue Feb 2, 2020
@bricelam bricelam removed the good first issue This issue should be relatively straightforward to fix. label Mar 11, 2020
@ajcvickers ajcvickers removed this from the 5.0.0 milestone Mar 12, 2020
@ajcvickers ajcvickers added this to the Backlog milestone Mar 12, 2020
@ajcvickers ajcvickers changed the title migrations are too aggressive on RenameColumn Mmigrations are too aggressive on RenameColumn Oct 10, 2020
@schuettecarsten
Copy link

Maybe add an "OldColumnName" attribute to tell EFCore that a rename is useful when a migration is created...?

@roji roji changed the title Mmigrations are too aggressive on RenameColumn Migrations are too aggressive on RenameColumn Sep 8, 2021
@304NotModified
Copy link

Maybe add an option to add-migration, e.g. -no-renames?

@jeff-pang
Copy link

jeff-pang commented Oct 22, 2022

Suggestion:
Add a new type of migration file "CustomMigration"

public partial class MyRenameCustomMigration : CustomMigration
{
    protected override void Up(CustomMigrationBuilder migrationBuilder)
    {
            migrationBuilder.RenameColumn(
                name: "FullName",
                schema: "sample",
                table: "Customers",
                newName: "FirstName");

            migrationBuilder.DoNotRename(
                name: "EntityAId",
                schema: "sample",
                table: "Customer");
}

migrations add RefactorCustomer --WithCustomMigration MyRenameCustomMigration

Ef migrations will generate other changes as usual but not touch the column names touched by the CustomMigration and then merge all changes into the migration file i.e RefactorCustomer

Future IDE tooling can also opt to include helpers to generate CustomMigration files with user input when performing class refactoring via IDE

@DanielPeinhopf
Copy link

I have another idea:
Why doesn't EF allow to add a DesignIdentity-attribute to entities and properties?
Eg.

[DesignIdentity("260d66ca-7e72-48cb-978d-c5d18f13bd67")]
public string? Name { get;set; }

This design identity could also be written to the column in the database by using extended properties on the column for example.
As long as the design identity stays the same, EF would know that it's still the same property and renaming it or changing it's type could result in the correct RENAME, ALTER COLUMN statements.

@jeff-pang
Copy link

jeff-pang commented Oct 27, 2022

I have another idea: Why doesn't EF allow to add a DesignIdentity-attribute to entities and properties? Eg.

[DesignIdentity("260d66ca-7e72-48cb-978d-c5d18f13bd67")]
public string? Name { get;set; }

This design identity could also be written to the column in the database by using extended properties on the column for example. As long as the design identity stays the same, EF would know that it's still the same property and renaming it or changing it's type could result in the correct RENAME, ALTER COLUMN statements.

DesignIdentity is a good solution in situation where the models is within the control of the developer and also the willingness to use annotate the models with [Attributes]. But this solution doesn't work very well in large project and teams where models are shared amongst teams and projects and tightly controlled. In this case annotating classes may or may not be in developer control also there may be large number of properties and classes to annotate.

@ajcvickers ajcvickers removed this from the Backlog milestone Jan 6, 2023
@ajcvickers
Copy link
Contributor

ajcvickers commented Jan 6, 2023

Consider also not renaming if multiple properties are equal candidates for the rename. See #29902.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants