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

Allow defining column order via [ColumnAttribute.Order] when creating tables #10059

Closed
Tracked by #22946 ...
bricelam opened this issue Oct 12, 2017 · 44 comments · Fixed by #25942
Closed
Tracked by #22946 ...

Allow defining column order via [ColumnAttribute.Order] when creating tables #10059

bricelam opened this issue Oct 12, 2017 · 44 comments · Fixed by #25942
Assignees
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity punted-for-3.0 punted-for-5.0 type-enhancement
Milestone

Comments

@bricelam
Copy link
Contributor

#2272 matched the CLR reflection order for columns within a table.

During design, we decided to wait for more feedback before honoring the value of ColumnAttibute.Order on the properties.

If the implementation of #2272 is insufficient for you and specifying something like [Column(Order = 1)] would help, please vote for this issue and add details about your scenario (if not already listed) below.

@ajcvickers ajcvickers added this to the Backlog milestone Oct 13, 2017
@yitzchokneuhaus1
Copy link

yitzchokneuhaus1 commented May 9, 2018

@bricelam Is this enhancement scheduled for the near future to be implemented in either EF Core or EF6?

@bricelam
Copy link
Contributor Author

bricelam commented May 9, 2018

It works in EF6. Still on the Backlog of EF Core.

@samconn
Copy link

samconn commented May 30, 2018

+1 for implementing this feature, and sooner rather than later.

We're porting an SAAS application from EF6x to EFCore and were a wee bit taken aback to find that the Order designation on the ColumnAttribute is being ignored.

But kudos to the whole team (and community!) for at least going open-kimono on the design discussions to at least help us understand the reasoning.

@bricelam
Copy link
Contributor Author

@andytaw mentioned another good case for this:

Column order is of no great significance if all data operations go through EF.
I need to use SQL Bulk Copy as well as EF.
I could (will...) write a whole bunch of tedious code to manage the two, however I'd much prefer to use .HasColumnOrder().

@MelGrubb
Copy link

MelGrubb commented Jul 12, 2018

Those who work in the database all day tend like their columns in decreasing order of "importance", with the most significant or often-used coming at the start, and picky details way over on the right out of the way, or sometimes they like them clustered in neighborhoods where related fields are next to each other. Either way, this means that the column order can be significant to some team members. Meanwhile, over in the land of code, I like to use tools like ReSharper or CodeMaid to automatically sort my C# code, which drastically reduces merge collisions. Really, I can't recommend it enough for team projects, it's a huge gain in productivity.

As it stands now, I can't make both sets of developers happy. I really need a way to specify the order of the database columns, and it seems to be that using the existing DataAnnotations' ColumnAttribute would be an ideal way to do this.

As for strategy, I'd recommend putting the properties with the attribute first, in increasing order according to the Order property of the attribute, and then the remaining properties in their existing "natural" order, although that order is non-deterministic and shouldn't be relied upon to remain stable in the future.

@pwen090
Copy link

pwen090 commented Sep 20, 2018

@bricelam you mentioned it on #2272 but to reiterate and upvote here if we have something like:

public class Blog
{
    public int BlogId { get; set; }
    public string Url { get; set; }
}

public class RepoBlog : Blog
{
    [Key]
    public int Id { get; set; }
}

This will cause the primary key Id to be output as the last column. I know we can edit migration files but surely somewhere along the lines someone will forget to do that and make it a pain to clean up. It would be great if we can either support Order attribute or at least have the generators try harder to always put key columns first.

@bricelam
Copy link
Contributor Author

@pwen090 Tracked by #11314 (comment)

@smitpatel
Copy link
Member

Afaik the model posted by @pwen090 wouldn't work if Blog is mapped since derived types cannot have keys.

@davisnw
Copy link

davisnw commented Apr 3, 2019

Is there any workaround right now before this gets implemented? HasColumnOrder doesn't seem to be supported either, and it makes it a real pain to check that the mappings are in sync with a database that is not primarily under the control of EntityFramework. E.g, generate database with e.g. EnsureCreated and doing a schema comparison with SSDT - but if I can't control EF column order, the comparisons don't line up. I was thinking there might be something in the metadata api, but I haven't found it yet if it exists.

@ajcvickers
Copy link
Member

@davisnw I'm not aware of any workaround for using EnsureCreated directly. The normal workaround is to scaffold a migration and arrange the column order in the migration.

@kayakingcoder
Copy link

As for strategy, I'd recommend putting the properties with the attribute first, in increasing order according to the Order property of the attribute, and then the remaining properties in their existing "natural" order, although that order is non-deterministic and shouldn't be relied upon to remain stable in the future.

This would not work in our scenario where we have LastModifiedDate and LastModifiedUserId as properties in a base class. I'd want a way to put those two fields at the end of my tables (as per the "importance" that @MelGrubb mentions) without being forced to put an order attribute on every property of descendant classes, instead they currently appear straight after the id.

I would prefer this strategy from https://linq2db.github.io/api/LinqToDB.Mapping.ColumnAttribute.html
"Specifies the order of the field in table creation. Positive values first (ascending), then unspecified (arbitrary), then negative values (ascending)."

@MelGrubb
Copy link

Couldn't you just put attributes with ridiculously high values on the base classes?

@maulik-modi
Copy link

@Fosol , I agree with you. This becomes important for reporting tables where there are 30-40 columns.
@Fosol Have you checked Nhibernate?
@roji , Is there any possibility of doing in EF Core 7?

@roji
Copy link
Member

roji commented Sep 4, 2021

@maulik-modi at the moment, the plan is still to get it in for 6.0.

@Fosol
Copy link

Fosol commented Sep 4, 2021

@maulik-modi I have looked at Nhibernate. It's a possible option for some work, but not in the current projects I'm engaged on.

bricelam added a commit to bricelam/efcore that referenced this issue Sep 8, 2021
@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 Sep 8, 2021
@bricelam bricelam modified the milestones: 6.0.0, 6.0.0-rc2 Sep 8, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Sep 10, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Sep 11, 2021
@Dzivo
Copy link

Dzivo commented Sep 26, 2021

Ordering doesnt work if you have following structure:

  public class EntityBase<TEntity> : IEntityBase<TEntity>
    where TEntity : class, IEntityBase<TEntity>
  {
    [Key, Column(Order = 0), DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public virtual int Id { get; set; }

    public Func<TEntity, object[]> PrimaryKey()
    {
      return m => new object[] { m.Id };
    }
  }
  public class TenantEntityBase<TEntity> : EntityBase<TEntity>, ITenantEntity
    where TEntity : class, IEntityBase<TEntity>
  {
    [Column(Order = 1), DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public virtual Guid TenantId { get; set; }
  }
public class MyClass: TenantEntityBase<MyClass>
  {
    public string Name { get; set; }
 }

Generated Migration:

migrationBuilder.CreateTable(
    name: "MyClass",
    columns: table => new
    {
        Id = table.Column<int>(type: "int", nullable: false)
            .Annotation("SqlServer:Identity", "1, 1"),
        Name = table.Column<string>(type: "nvarchar(max)", nullable: true),
        ZLastProp = table.Column<string>(type: "nvarchar(max)", nullable: true),
        TenantId = table.Column<Guid>(type: "uniqueidentifier", nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_MyClass", x => x.Id);
    });

@TanvirArjel
Copy link

@Dzivo Please add the migration output here.

@roji
Copy link
Member

roji commented Sep 26, 2021

@Dzivo better yet, please open a new issue with the full details.

@Dzivo
Copy link

Dzivo commented Sep 26, 2021

Just a sec i will make new project fast to see if it happens there

@Dzivo
Copy link

Dzivo commented Sep 26, 2021

Here is whole example:

https://we.tl/t-cQLQ3E0pOA

it uses local docker sql server version
docker pull mcr.microsoft.com/mssql/server

But you can change connection string in startup.cs

All classes are defined in Context.cs

@Dzivo
Copy link

Dzivo commented Sep 29, 2021

Did you take a look at it ? Thoughts ?

@roji
Copy link
Member

roji commented Sep 30, 2021

@Dzivo I'll take a look, but can you please open a new issue and post the repro there?

@ajcvickers
Copy link
Member

@Dzivo Your project uses EF Core 5.0.9. This issue is fixed in EF Core 6.0 RC2 as indicated by its milestone, which will be released soon, or you can get it from a daily build.

@anshuldubey
Copy link

anshuldubey commented Apr 8, 2022

The column ordering issue is still there for UPDATE statements. When we try to perform UPDATE operation from EFCore, it changes the order of primary keys alphabetically. That's strange because SELECT query executed from EFCore retains the correct order.

Using EF Version: 5.0.2

@roji
Copy link
Member

roji commented Apr 11, 2022

@anshuldubey this issue is about the order of columns in the table, when they're created via migrations - not about the order in which columns are specified in SELECT or UPDATE statements. I'm not aware of any importance to column order in SELECT/UPDATE; if you feel there's a problem, please open a new issue, with an explanation of why you think the any change is needed.

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. ef6-parity punted-for-3.0 punted-for-5.0 type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.