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

Required column convert to nullable in case of property name matches with table name #30937

Open
sa-es-ir opened this issue May 20, 2023 · 4 comments

Comments

@sa-es-ir
Copy link

I have an entity with these property:

public class Customer
{
    [Key]
    public long CustomerKey { get; set; }

    [Required]
    [MaxLength(20)]
    public string CustomerId { get; set; }
}

If I add migration here is the generated one:

migrationBuilder.CreateTable(
                name: "Customers",
                columns: table => new
                {
                    CustomerKey = table.Column<long>(type: "bigint", nullable: false)
                        .Annotation("SqlServer:Identity", "1, 1"),
                    CustomerId = table.Column<string>(type: "nvarchar(20)", maxLength: 20, nullable: true)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Customers", x => x.CustomerKey);
                });

Which set the CustomerId as nullable, if I use fluent api it's ok and will make it required.
Note that in the EF 5.0, it works as expected but EF 6.0 and 7.0 do not work.

Include provider and version information

EF Core version: 6.0.0 and 7.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0

@roji
Copy link
Member

roji commented May 20, 2023

I am unable to reproduce this - copying and pasting your code and creating a migration produces the following, with nullable: false:

migrationBuilder.CreateTable(
    name: "Customers",
    columns: table => new
    {
        Id = table.Column<int>(type: "int", nullable: false)
            .Annotation("SqlServer:Identity", "1, 1"),
        CustomerId = table.Column<string>(type: "nvarchar(20)", maxLength: 20, nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_Customers", x => x.Id);
    });

Following is the minimal, runnable code sample I've used based on your code - please produce a similar sample (or tweak the below) to show the error happening.

Attempted repro
await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

public class BlogContext : DbContext
{
    public DbSet<Customer> Customers { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(
                @"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
}

public class Customer
{
    [Key]
    public int Id { get; set; }

    [Required]
    [MaxLength(20)]
    public string CustomerId { get; set; }
}

@sa-es-ir
Copy link
Author

@roji Thanks for the response,
Here is my simple project that can reproduce the issue
https://github.com/sa-es-ir/EFTestRequiredColumn

I already generated the migrations but you can remove the migrations and recreate them.

@ajcvickers
Copy link
Member

ajcvickers commented May 22, 2023

Note for triage: still repros on latest daily build. Presumably, when CustomerId is changed from not being the key, its requiredness is also changed, even though it should not be since it was configured by data annotation.

@sa-es-ir Workaround (Edit: just noticed you already said that! :-)) is to configure the property as required in the fluent API:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Customer>().Property(e => e.CustomerId).IsRequired();
}

@ajcvickers
Copy link
Member

Note from triage: likely fixed by layering (#15898).

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

No branches or pull requests

3 participants