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: Order columns of abstract base class properties last in CreateTable #11314

Closed
TanvirArjel opened this issue Mar 17, 2018 · 61 comments
Assignees
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. poachable punted-for-2.1 punted-for-3.0 type-enhancement
Milestone

Comments

@TanvirArjel
Copy link

TanvirArjel commented Mar 17, 2018

Before the release of Entity Framework Core 2.1, Entity Framework Core team announced that,

Column ordering in migrations: Based on customer feedback, we have updated migrations to initially generate columns for tables in the same order as properties are declared in classes.

but unfortunately, it is not working as expected in case of the following scenarios:

public abstract class AuditModel
{
    public int? CreatedBy { get; set; }
    public DateTime CreatedAt { get; set; } = DateTime.UtcNow;
    public int? ModifiedBy { get; set; }
    public DateTime? ModifiedAt { get; set; }
    public bool IsActive { get; set; } = true;
}

public class Employee : AuditModel
{
   [Key]
   [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
   public long EmployeeId { get; set; }

   public string EmployeeNo { get; set; }

   [ForeignKey("Company")]
   public long CompanyId { get; set; }

   [ForeignKey("CompanyOffice")]
   public long OfficeId { get; set; }

   [ForeignKey("Section")]
   public long SectionId { get; set; }
    ..........................
}

Entity Framework Core Migration is generating the Employee Table columns as the following order:

  [CreatedBy] <-----
  ,[CreatedAt] <-----
  ,[ModifiedBy] <-----
  ,[ModifiedAt] <-----
  ,[IsActive] <-----
  ,[EmployeeId]
  ,[EmployeeNo]
  ,[CompanyId]
  ,[OfficeId]
  ,[SectionId]
  ............

Actually It should have been:

  ,[EmployeeId]
  ,[EmployeeNo]
  ,[CompanyId]
  ,[OfficeId]
  ,[SectionId]
   ............
  ,[CreatedBy] <-----
  ,[CreatedAt] <-----
  ,[ModifiedBy] <-----
  ,[ModifiedAt] <-----
  ,[IsActive] <-----

Note that, Entity Framework 6.x Migration generates the columns as expected order even in case of inheritance.

@TanvirArjel TanvirArjel changed the title Column order broken in migration while class inheriting few column sfrom another class Column order broken in migration while model class inheriting few columns from another class Mar 17, 2018
@ajcvickers
Copy link
Member

@TanvirArjel Can you be more specific about what is "wrong" with the order generated? For example, is it just that the primary key is not the first column? Or is it also that the unmapped base type should go at the end? If the latter, then what is the logic that determines this and would that also be true if it was a mapped base type?

@TanvirArjel
Copy link
Author

@ajcvickers Yes! Primary key should be the first column and the unmapped based type columns should go at the end..

@ajcvickers
Copy link
Member

@TanvirArjel Can you provide an explanation as to why the unmapped base type should go at the end and whether or not this should also be the case for a mapped base type? (I'm not saying you're wrong, I'm just trying to understand what the reasoning is behind this ordering.)

@TanvirArjel
Copy link
Author

Dear @ajcvickers , Thanks for your persistent reply.

Does it make sense that the primary key of the table at column no. 5 or so other than the first column of the table? Look at the two orders of the columns in the question. Which makes sense? Which is more meaningful? Obviously not the first order..Hope,you will also be agree with me.

In case of unmapped based type, yes! it also be more readable and rational that unmapped based type columns should go at the end of the table because normally in the common unmapped base type we put the columns which are common to the every table like CreatorName, CreationTime, ModifierName, ModificationTime etc. These columns at the beginning of the table does not make sense.

Point to be noted that, In Entity Framework 6.2.0, Primary key of the table is always the first column and both unmapped and mapped based type goes at the end. This really makes sense!

I know Entity Framework Core is a breaking change from Entity Framework but what's wrong to bring these already accustomed,well tested and more sensible behaviors from Entity Framework to Entity Framework Core?

@ajcvickers
Copy link
Member

@TanvirArjel Thanks for the additional information. It will help us decide our course of action here.

@ajcvickers
Copy link
Member

Triage decisions:

  • Primary keys first
  • Un-mapped base classes at end

@TanvirArjel
Copy link
Author

Dear @ajcvickers, Thanks a lot! for taking speedy action..That's why we love and rely on Microsoft :)

@pklejnowski
Copy link

Is there any workaround for this?

@tehZeno
Copy link

tehZeno commented Dec 28, 2019

Is there any workaround for this?

I manually edited the files and changed the order; but it's a royal pain in the behind.
Don't think there is a real workable workaround yet.

@administersoftware
Copy link

administersoftware commented Dec 29, 2019

Is there any workaround for this?

I'm still manually edit the migration file, and as @tehZeno says, it's a royal pain in the behind :(

@drewpayment
Copy link

drewpayment commented Dec 30, 2019

Is there any workaround for this?

This is wild. There are a multitude of issues that have been opened over the course of the last five years, closed for various reasons and ultimately hundreds, or possibly thousands, of hours wasted because no one has just fixed this issue.

I was thinking about using EF Core, but I mean......... this is painful after the last 30 minutes of reading through these issues.

@administersoftware
Copy link

Is there any workaround for this?

This is wild. There are a multitude of issues that have been opened over the course of the last five years, closed for various reasons and ultimately hundreds, or possibly thousands, of hours wasted because no one has just fixed this issue.

I was thinking about using EF Core, but I mean......... this is painful after the last 30 minutes of reading through these issues.

@drewpayment I definitely agree to the number of hours wasted on this one issue, but I also understand that there are more important issues for the team to be focussed on, what we need is more people to upvote the priority of this issue. Atleast the issue remains open :)

@Luis-Palacios
Copy link

In any case for those of you that like me are manually ordering the migrations, have you found a way to order columns when adding a new column to an existing table?

@TanvirArjel
Copy link
Author

@Luis-Palacios No way! You have to do it manually.

@premchandrasingh
Copy link

I managed ordering by customizing SqlServerMigrationsAnnotationProvider and SqlServerMigrationsSqlGenerator. Even though it says internal API, I think it is much cleaner until available out of the box. Here is git repo https://github.com/premchandrasingh/EFCoreColumnOrder

@DesarrolloNedugaTech
Copy link

I tried it and it works, thanks and regards.

@Seabizkit
Copy link

can this be fixed already we are on version 3.1 of ef core already. I would also like to see the "index" attribute back. like ef 6 had. I wouldn't think it would be that hard to implement order by on attributes for migrations.

@bricelam
Copy link
Contributor

bricelam commented Feb 26, 2020

Yes, the plan is to fix this in the next version of EF Core. Here is the issue for an index attribute: #4050. I'm not sure what you mean by order by on attributes for migrations, but please always feel free to submit a PR; all six team members are eager to help users fix their biggest pet peeves or implement small enhancements.

@zgonzales
Copy link

zgonzales commented May 12, 2020

Please fix this soon, it is pretty high on the list when sorting the issues by most comments.

@Seabizkit
Copy link

@bricelam FYI just in case it wasn't clear

There are two attributes....

  • "[Index(name), order, IsUnique = true)]"
  • "[Column(Order = 601)]"

What we are wanting is the order they are specified in the model.
when there is an abstract class inherited then they must be appended to the end except for the table key.

We also want to be able to specify preference.... so giving a lower Order will make it appear closer to the start. Basically what EF6 is doing with the added ability to specify higher or lower index on abstract classes.

This issue is more about the ordering than the Index, just sharing as you gave a link to the missing Index one, but the "bigger" issue is the order.

Index can be specified using fluent notation.

class entity
{
    Id 
    [Column(Order = 2)]
    DateCreated

    DateModfied
}

class persion : entity
{
     Name
     Email
}

the order

Id
DateCreated
Name
Email
DateModfied

@bricelam
Copy link
Contributor

Nobody here has any expectations about unmapped types in the middle of a hierarchy, right? This is just about the case identified in the issue description?

@bricelam
Copy link
Contributor

bricelam commented Aug 10, 2020

Also, if an owned type has an unmapped base type, it's properties should go next to the other owned type's properties still, right?

bricelam added a commit to bricelam/efcore that referenced this issue Aug 10, 2020
@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 10, 2020
@TanvirArjel
Copy link
Author

Nobody here has any expectations about unmapped types in the middle of a hierarchy, right?

Yes! but if anybody wants this then he/should be able to use ColumnOrderAttribute to specify the custom position,

@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-rc1 Aug 14, 2020
@aalsamawi
Copy link

I managed ordering by customizing SqlServerMigrationsAnnotationProvider and SqlServerMigrationsSqlGenerator. Even though it says internal API, I think it is much cleaner until available out of the box. Here is git repo https://github.com/premchandrasingh/EFCoreColumnOrder

This doesn't work with MySQL. Any thoughts?
here's the error I'm getting:

System.InvalidOperationException: The current migration SQL generator 'CustomSqlServerMigrationsSqlGenerator' is unable to generate SQL for operations
of type 'Microsoft.EntityFrameworkCore.Migrations.Operations.MySqlCreateDatabaseOperation'.
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.Generate(MigrationOperation operation, IModel model, MigrationCommandListBuilder
builder)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.Generate(IReadOnlyList`1 operations, IModel model)
   at Pomelo.EntityFrameworkCore.MySql.Storage.Internal.MySqlDatabaseCreator.CreateCreateOperations()
   at Pomelo.EntityFrameworkCore.MySql.Storage.Internal.MySqlDatabaseCreator.Create()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Pomelo.EntityFrameworkCore.MySql.Migrations.Internal.MySqlMigrator.Migrate(String targetMigration)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabaseImpl(String targetMigration, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabase.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
The current migration SQL generator 'CustomSqlServerMigrationsSqlGenerator' is unable to generate SQL for operations of type 'Microsoft.EntityFrameworkCore.Migrations.Operations.MySqlCreateDatabaseOperation'.

@pgiacomo69
Copy link

pgiacomo69 commented Oct 16, 2020

Today, with Net 5.0 RC2 and Pomelo.EntityFrameworkCore.MySql:

public partial class BaseEntity
{
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
[Column("ID", Order = 0)]
public long Id { get; set; }
[Column("CREATED_AT", Order = 995)]
[IgnoreDataMember]
[JsonIgnore]
public DateTime? CreatedAt { get; set; }
[Column("CREATED_BY_SESSION_ID", Order = 996)]
[IgnoreDataMember]
[JsonIgnore]
public long CreatedBySessionId { get; set; }
[Column("UPDATED_AT", Order = 997)]
[IgnoreDataMember]
[JsonIgnore]
public DateTime? UpdatedAt { get; set; }
[Column("UPDATED_BY_SESSION_ID", Order = 998)]
[IgnoreDataMember]
[JsonIgnore]
public long UpdatedBySessionId { get; set; }
[Column("VERSION_ID", Order = 999)]
public long? VersionId { get; set; }
}

[Table("Tb_Geo_Zone")]
public class GeoZona :BaseEntity
{
[Required]
[Column("codice", Order = 1)]
[StringLength(10)]
public string codice { get; set; }

    [Column("nome", Order = 2)]
    [StringLength(10)]
    public string Nome { get; set; }
}

Sql autogenerated from Migration:

CREATE TABLE Tb_Geo_Zone (
ID bigint NOT NULL AUTO_INCREMENT,
codice varchar(10) CHARACTER SET utf8mb4 NOT NULL,
nome varchar(10) CHARACTER SET utf8mb4 NULL,
CREATED_AT datetime(6) NULL,
CREATED_BY_SESSION_ID bigint NOT NULL,
UPDATED_AT datetime(6) NULL,
UPDATED_BY_SESSION_ID bigint NOT NULL,
VERSION_ID bigint NULL,
CONSTRAINT PK_Tb_Geo_Zone PRIMARY KEY (ID),
CONSTRAINT AK_Tb_Geo_Zone_codice UNIQUE (codice)
);

Now it works!!!!

@ajcvickers ajcvickers modified the milestones: 5.0.0-rc1, 5.0.0 Nov 7, 2020
@skini82
Copy link

skini82 commented Mar 10, 2022

I'm having the same issue in EF Core 6.0.3 on VS2022 with SQLServer DB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. poachable punted-for-2.1 punted-for-3.0 type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.