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: Column order incorrect in CreateTable when using interfaces #11727

Closed
HEBOS opened this issue Apr 18, 2018 · 17 comments
Closed

Migrations: Column order incorrect in CreateTable when using interfaces #11727

HEBOS opened this issue Apr 18, 2018 · 17 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-2.2 punted-for-3.0 punted-for-3.1 type-bug
Milestone

Comments

@HEBOS
Copy link

HEBOS commented Apr 18, 2018

With respect to pull request that is dealing with issue and my duplicate, I can report that it indeed creates proper "CreateTable" code for InitialCreate.cs, but at the same time the code for InitialCreate.Designer.cs is not properly created.
This causes table not to be created with proper column order.

This is InitialCreate.cs output (and order of columns is correct):

migrationBuilder.CreateTable(
                name: "UserTask",
                schema: "Configurations",
                columns: table => new
                {
                    Id = table.Column<int>(nullable: false)
                        .Annotation("SqlServer:ValueGenerationStrategy", SqlServerValueGenerationStrategy.IdentityColumn),
                    ClientId = table.Column<byte>(nullable: false),
                    ModuleId = table.Column<int>(nullable: false),
                    TaskId = table.Column<int>(nullable: false),
                    UserId = table.Column<int>(nullable: false),
                    ModifiedBy = table.Column<string>(maxLength: 100, nullable: false),
                    LastModification = table.Column<DateTime>(type: "datetime", nullable: false)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_UserTask", x => x.Id);
                });

This is "InitialCreate.Designer.cs" output (and order of columns is NOT correct):

modelBuilder.Entity("MongoPlus.Domain.Models.Subscription.Configurations.UserTask", b =>
                {
                    b.Property<int>("Id")
                        .ValueGeneratedOnAdd();

                    b.Property<byte>("ClientId");

                    b.Property<DateTime>("LastModification")
                        .HasColumnType("datetime");

                    b.Property<string>("ModifiedBy")
                        .IsRequired()
                        .HasMaxLength(100);

                    b.Property<int>("ModuleId");

                    b.Property<int>("TaskId");

                    b.Property<int>("UserId");

                    b.HasKey("Id");

                    b.ToTable("UserTask","Configurations");
                });

When I run "Update-Database -Context SubscriptionDbContext", I get table created with column order as coded in "InitialCreate.Designer.cs", and that means that that issue should be reopened.

Further technical details

EF Core version: 2.1.0 Preview 2
Database Provider: Microsoft SQL
Operating system: Windows 10
IDE :Visual Studio 2017 15.7 Preview

@ajcvickers
Copy link
Member

@HEBOS The order of columns in the designer file should not impact that column order in the database. Please post a runnable project/solution or full code listing that demonstrates the behavior you are seeing so that we can investigate.

@HEBOS
Copy link
Author

HEBOS commented Apr 18, 2018

@ajcvickers

I've attached a sample project.

Note that migrations should be executed in Package Manager Console while "Data" project is selected as default. The main project is "TestColumnOrdering" project.

I've added sample migrations commands that I've used to the TestColumnOrdering\Program.cs file, so you can copy it from there.

Generated AccountingDocument table has columns incorrectly ordered.

Thank you

TestColumnOrderingSolution.zip

@ajcvickers
Copy link
Member

@HEBOS Thanks for the repro. It looks like AccountingDocument is indeed losing the order, while AccountingDocumentDetail is retaining order. Assigning to @bricelam to investigate.

CREATE TABLE [Finance].[AccountingDocument] (
    [ClientFiscalYearId] int NOT NULL,
    [ClientId] tinyint NOT NULL,
    [Date] date NOT NULL,
    [DocumentTypeId] int NOT NULL,
    [Id] int NOT NULL,
    [LastModification] datetime NOT NULL,
    [Note] nvarchar(max) NOT NULL,
    [ModifiedBy] nvarchar(100) NOT NULL,
    CONSTRAINT [PK_AccountingDocument] PRIMARY KEY ([Id], [ClientId], [ClientFiscalYearId])
);
CREATE TABLE [Finance].[AccountingDocumentDetail] (
    [Id] int NOT NULL IDENTITY,
    [AccountingDocumentId] int NOT NULL,
    [ClientId] tinyint NOT NULL,
    [ClientFiscalYearId] int NOT NULL,
    [AccountId] int NOT NULL,
    [PartnerId] nvarchar(15) NULL,
    [CostCenterId] nvarchar(15) NULL,
    [DocumentReference] nvarchar(100) NULL,
    [Description] nvarchar(100) NULL,
    [Date] date NOT NULL,
    [DueDate] date NOT NULL,
    [Debit] decimal(14, 2) NOT NULL,
    [Credit] decimal(14, 2) NOT NULL,
    [ForeignCurrency] nvarchar(3) NOT NULL,
    [DebitInForeignCurrency] decimal(14, 2) NOT NULL,
    [CreditInForeignCurrency] decimal(14, 2) NOT NULL,
    [PaymentReferenceModel] nvarchar(4) NULL,
    [PaymentReference] nvarchar(100) NULL,
    [ModifiedBy] nvarchar(100) NOT NULL,
    [LastModification] datetime NOT NULL,
    CONSTRAINT [PK_AccountingDocumentDetail] PRIMARY KEY ([Id]),
    CONSTRAINT [FK_AccountingDocumentDetail_AccountingDocument] FOREIGN KEY ([AccountingDocumentId], [ClientId], [ClientFiscalYearId]) REFERENCES [Finance].[AccountingDocument] ([Id], [ClientId], [ClientFiscalYearId]) ON DELETE NO ACTION
);

@ajcvickers
Copy link
Member

@bricelam FYI talked to @divega and we're thinking that we should not take this for 2.1. (Didn't want you to think you needed to work on it tonight.)

@HEBOS
Copy link
Author

HEBOS commented Apr 19, 2018

Hi,

Is this a major issue?
I'm currently working on a development of a cloud solution, and this will take me a couple of months, but after that, I need to have it for sure.
Do you have some estimate on this?

Thanks

@ajcvickers
Copy link
Member

ajcvickers commented Apr 19, 2018

@HEBOS It will likely get triaged into the 2.2 release, for which there are no public dates available. It doesn't meet the bar in terms of severity to go into 2.1 RC at this point, especially since there is a workaround of manually ordering the columns in the initial migration.

@HEBOS
Copy link
Author

HEBOS commented Apr 19, 2018

@ajcvickers
I understand your point. Even though, imagine someone with 100 tables (like me) to do that manual sorting.
I hope it will be in the next release though.
Thanks for the update.

@bricelam
Copy link
Contributor

I suspect the interfaces are throwing off the algorithm. The properties on them are first, and the rest are alphabetical which means it didn't find the CLR type.

@MelGrubb
Copy link

MelGrubb commented Jul 9, 2018

I would prefer a system in which I can directly control the column order, similarly to how I can use the .Totable or .HasIndex methods now. I see some discussion about a .HasColumnOrder, but I don't see it in the released 2.1

This is important to me because I run automatic code cleanup, including member sorting, on all my projects. It results in a drastic reduction in merge conflicts, which makes working together with a team much more pleasant. It's not necessarily how I (or the resident DBA) want the columns ordered in the table, though. I really need to decouple these concepts.

@bricelam
Copy link
Contributor

bricelam commented Jul 9, 2018

@MelGrubb See #10059

@bricelam bricelam modified the milestones: Backlog, 3.1.0 Sep 4, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, Backlog Oct 11, 2019
@bricelam bricelam modified the milestones: Backlog, 5.0.0 Nov 5, 2019
@bricelam bricelam changed the title Migrations: Use reflection order for columns in CreateTable - Not Working Migrations: Column order incorrect in CreateTable when using interfaces Dec 10, 2019
@lajones lajones assigned lajones and unassigned bricelam Feb 25, 2020
@lajones
Copy link
Contributor

lajones commented Feb 25, 2020

Poaching. (Note: just the part about making it work with interfaces for now).

@bricelam
Copy link
Contributor

Related issues: #11314 and #10059

@lajones lajones removed the poachable label Feb 27, 2020
@ajcvickers
Copy link
Member

@lajones So I was able to reproduce this with the customer's attached project. However, updating this project to use 3.1.2 it no longer repros--see generated migration below. So when you said you could repro it outside of the EF build, did you actually try with the 3.1.2 release?

protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.EnsureSchema(
        name: "Finance");

    migrationBuilder.CreateTable(
        name: "AccountingDocument",
        schema: "Finance",
        columns: table => new
        {
            Id = table.Column<int>(nullable: false),
            ClientId = table.Column<byte>(nullable: false),
            ClientFiscalYearId = table.Column<int>(nullable: false),
            Date = table.Column<DateTime>(type: "date", nullable: false),
            DocumentTypeId = table.Column<int>(nullable: false),
            Note = table.Column<string>(nullable: false),
            ModifiedBy = table.Column<string>(maxLength: 100, nullable: false),
            LastModification = table.Column<DateTime>(type: "datetime", nullable: false)
        },
        constraints: table =>
        {
            table.PrimaryKey("PK_AccountingDocument", x => new { x.Id, x.ClientId, x.ClientFiscalYearId });
        });

    migrationBuilder.CreateTable(
        name: "AccountingDocumentDetail",
        schema: "Finance",
        columns: table => new
        {
            Id = table.Column<int>(nullable: false)
                .Annotation("SqlServer:Identity", "1, 1"),
            AccountingDocumentId = table.Column<int>(nullable: false),
            ClientId = table.Column<byte>(nullable: false),
            ClientFiscalYearId = table.Column<int>(nullable: false),
            AccountId = table.Column<int>(nullable: false),
            PartnerId = table.Column<string>(maxLength: 15, nullable: true),
            CostCenterId = table.Column<string>(maxLength: 15, nullable: true),
            DocumentReference = table.Column<string>(maxLength: 100, nullable: true),
            Description = table.Column<string>(maxLength: 100, nullable: true),
            Date = table.Column<DateTime>(type: "date", nullable: false),
            DueDate = table.Column<DateTime>(type: "date", nullable: false),
            Debit = table.Column<decimal>(type: "decimal(14, 2)", nullable: false),
            Credit = table.Column<decimal>(type: "decimal(14, 2)", nullable: false),
            ForeignCurrency = table.Column<string>(maxLength: 3, nullable: false),
            DebitInForeignCurrency = table.Column<decimal>(type: "decimal(14, 2)", nullable: false),
            CreditInForeignCurrency = table.Column<decimal>(type: "decimal(14, 2)", nullable: false),
            PaymentReferenceModel = table.Column<string>(maxLength: 4, nullable: true),
            PaymentReference = table.Column<string>(maxLength: 100, nullable: true),
            ModifiedBy = table.Column<string>(maxLength: 100, nullable: false),
            LastModification = table.Column<DateTime>(type: "datetime", nullable: false)
        },
        constraints: table =>
        {
            table.PrimaryKey("PK_AccountingDocumentDetail", x => x.Id);
            table.ForeignKey(
                name: "FK_AccountingDocumentDetail_AccountingDocument",
                columns: x => new { x.AccountingDocumentId, x.ClientId, x.ClientFiscalYearId },
                principalSchema: "Finance",
                principalTable: "AccountingDocument",
                principalColumns: new[] { "Id", "ClientId", "ClientFiscalYearId" },
                onDelete: ReferentialAction.Restrict);
        });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocument_DocumentType",
        schema: "Finance",
        table: "AccountingDocument",
        columns: new[] { "DocumentTypeId", "ClientId" });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocumentDetail_AccountingDocument",
        schema: "Finance",
        table: "AccountingDocumentDetail",
        columns: new[] { "AccountingDocumentId", "ClientId", "ClientFiscalYearId" });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocumentDetail_Credit_ClientId_FiscalYearId",
        schema: "Finance",
        table: "AccountingDocumentDetail",
        columns: new[] { "Credit", "ClientId", "ClientFiscalYearId" });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocumentDetail_Date_ClientId_FiscalYearId",
        schema: "Finance",
        table: "AccountingDocumentDetail",
        columns: new[] { "Date", "ClientId", "ClientFiscalYearId" });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocumentDetail_Debit_ClientId_FiscalYearId",
        schema: "Finance",
        table: "AccountingDocumentDetail",
        columns: new[] { "Debit", "ClientId", "ClientFiscalYearId" });

    migrationBuilder.CreateIndex(
        name: "IX_AccountingDocumentDetail_DueDate_ClientId_FiscalYearId",
        schema: "Finance",
        table: "AccountingDocumentDetail",
        columns: new[] { "DueDate", "ClientId", "ClientFiscalYearId" });
}

protected override void Down(MigrationBuilder migrationBuilder)
{
    migrationBuilder.DropTable(
        name: "AccountingDocumentDetail",
        schema: "Finance");

    migrationBuilder.DropTable(
        name: "AccountingDocument",
        schema: "Finance");
}

@lajones
Copy link
Contributor

lajones commented Mar 4, 2020

No. I reproed it using the customer's project. I missed that the customer's project was still targeting 2.1.0. I see the same as you when I update. I think we can close this as no longer reproes then.

@ajcvickers
Copy link
Member

@lajones Please try to find a duplicate as I described in email.

@lajones lajones added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 10, 2020
@lajones lajones modified the milestones: 5.0.0, 3.1.x Mar 10, 2020
@lajones
Copy link
Contributor

lajones commented Mar 10, 2020

Can't find the specific commit that fixed it. But it was fixed some time before 3.1.2 was released.

@lajones lajones closed this as completed Mar 10, 2020
@smitpatel smitpatel modified the milestones: 3.1.x, 3.1.0 Mar 16, 2020
@smitpatel
Copy link
Member

@lajones - 3.1.x is milestone used for next patch release. Issues from 3.1.x are moved exact patch number milestone once merged to branch. Put something in patch milestone only if we know for sure it was fixed in particular patch else just use the major/minor version. In this case 3.1.0. If you don't see particular release in milestones menu then click on closed and you will find closed milestones there.

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

No branches or pull requests

7 participants