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

Did the naming convention change for HasCheckConstraint in v.6? Is this a breaking change? #27059

Closed
davemateer opened this issue Dec 22, 2021 · 4 comments · Fixed by #27142
Closed
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@davemateer
Copy link

Reproduce:

using Microsoft.EntityFrameworkCore;

namespace MigrationTest
{
    public sealed class Record
    {
        public int Id { get; set; }
    }

    public sealed class ApplicationDbContext : DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            base.OnConfiguring(optionsBuilder);
            optionsBuilder.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=test;Trusted_Connection=True;MultipleActiveResultSets=true");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);
            modelBuilder.Entity<Record>().HasCheckConstraint("CHK_Records_Id", "[Id] > 10");
        }

        public DbSet<Record> Records { get; set; }
    }

    class Program
    {
        static void Main(string[] args)
        {
        }
    }
}

When you generate a migration using dotnet ef migrations add foo using .Net 5 and EF Core 5.0.13 (libraries and tool), the migration output includes the following:

table.CheckConstraint("CHK_Records_Id", "[Id] > 10");

And the snapshot:

b.HasCheckConstraint("CHK_Records_Id", "[Id] > 10");

When you change everything to Net 6 and EF Core 6.0.0 (libraries and tool), those change to the following:

table.CheckConstraint("CK_Records_CHK_Records_Id", "[Id] > 10");

Snapshot:

b.HasCheckConstraint("CHK_Records_Id", "[Id] > 10");

Is this expected, or is this a bug?

@ajcvickers
Copy link
Contributor

Note for triage: verified that the constraint name is changed to "CK_Records_CHK_Records_Id" in 6.0.

Migration with 5.0:

using Microsoft.EntityFrameworkCore.Migrations;

namespace FiveOh.Migrations
{
    public partial class One : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.CreateTable(
                name: "Records",
                columns: table => new
                {
                    Id = table.Column<int>(type: "int", nullable: false)
                        .Annotation("SqlServer:Identity", "1, 1")
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Records", x => x.Id);
                    table.CheckConstraint("CHK_Records_Id", "[Id] > 10");
                });
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropTable(
                name: "Records");
        }
    }
}

Migration with 6.0:

using Microsoft.EntityFrameworkCore.Migrations;

#nullable disable

namespace Migrations
{
    public partial class One : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.CreateTable(
                name: "Records",
                columns: table => new
                {
                    Id = table.Column<int>(type: "int", nullable: false)
                        .Annotation("SqlServer:Identity", "1, 1")
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Records", x => x.Id);
                    table.CheckConstraint("CK_Records_CHK_Records_Id", "[Id] > 10");
                });
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropTable(
                name: "Records");
        }
    }
}

@AndriySvyryd
Copy link
Member

This is a known breaking change: https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-6.0/breaking-changes#unique-check-constraints
The store name needs to be specified using HasName:
modelBuilder.Entity<Record>().HasCheckConstraint("Id", "[Id] > 10", c => c.HasName("CHK_Records_Id"));

@davemateer
Copy link
Author

Ah, thanks. Sorry, I missed that in the docs. I even saw that section but I think my brain shut off because I wasn't doing anything that was related to uniqueness, so it didn't register that that mitigation applied.

At the end of the day, in our actual application, we're using IMutableEntityType.AddCheckConstraint which doesn't seem to have an overload that supports overriding the name like HasCheckConstraint does. (The name parameter passed to AddCheckConstraint is still going through the convention mutation apparently.) We ended up just going the "when in Rome" route and changed the name of all our constraints, but it's good to know this is expected behavior at least. Thanks for pointing that out.

@AndriySvyryd
Copy link
Member

@davemateer You can set IMutableCheckConstraint.Name, the name passed to AddCheckConstraint is actually ModelName.

We discussed this again and decided to revert the prefixing change, as it's unexpected and unnecessary.

AndriySvyryd added a commit that referenced this issue Jan 7, 2022
AndriySvyryd added a commit that referenced this issue Jan 7, 2022
AndriySvyryd added a commit that referenced this issue Jan 7, 2022
ajcvickers added a commit that referenced this issue Jan 10, 2022
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved labels Jan 10, 2022
@ajcvickers ajcvickers modified the milestones: 6.0.x, 6.0.2 Jan 10, 2022
ajcvickers added a commit that referenced this issue Jan 10, 2022
@AndriySvyryd AndriySvyryd removed their assignment Mar 15, 2024
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. customer-reported Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants