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

Validate that filters are the same for shared indexes #24968

Closed
ska737 opened this issue May 24, 2021 · 3 comments · Fixed by #25931
Closed

Validate that filters are the same for shared indexes #24968

ska737 opened this issue May 24, 2021 · 3 comments · Fixed by #25931
Assignees
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@ska737
Copy link

ska737 commented May 24, 2021

When using base class, attempting to be unique within the specific typed class does not apply the unique with Code-First.

Example excerpt:

public class TestContext : DbContext
{
  DbSet<ImplementClassOne> Ones { get; set; }
  DbSet<ImplementClassTwo> Twos { get; set; }

  protected void OnModelCreating(ModelBuilder builder)
  {
    modelBuilder.Entity<Base>().HasDiscriminator<string>("Type")
      .HasValue<ImplementClassOne>("one")
      .HasValue<ImplementClassTwo>("two");

    modelBuilder.Entity<Base>().Property(base => base.UniqueVal)
      .IsRequired();

    modelBuilder.Entity<Base>().Property(base => base.RequiredVal)
      .IsRequired();

    modelBuilder.Entity<ImplementClassOne>().HasIndex(one => one.UniqueVal)
      .IsUnique()
      .HasFilter("Type = 'one'");

    modelBuilder.Entity<ImplementClassTwo>().HasIndex(two => two.UniqueVal)
      .IsUnique()
      .HasFilter("Type = 'two'");
  }
}

This configuration creates the following Up() method in the Migration file, using Code-First:

protected override void Up(MigrationBuilder migrationBuilder)
{
  migrationBuilder.CreateTable(
    name: "Base",
    columns: table => new
    {
      Id = table.Column<int>(type: "int", nullable: false)
        .Annotation(...),
      UniqueVal = table.Column<string>(type: "nvarchar(max)", nullable: false,
      RequiredVal = table.Column<string>(type: "nvarchar(max)", nullable: false
    },
    constraints: table =>
    {
      table.PrimaryKey("PK_Base", x => x.Id);
    });

  migrationBuilder.CreateIndex(
    name: "IX_Base_UniqueVal_One",
    table: "Base",
    column: "UniqueVal",
    unique: true,
    filter: "Type = 'one'");
}

The second unique filtered index is missing. I can manually insert the index into the migration file. However, I am unable to test the second constraint when using normal unit testing methods, since unit tests normally do not use migrations.

Expected behavior would be for all unique filtered indexes be applied.

@ajcvickers
Copy link
Contributor

/cc @AndriySvyryd to take a look

@AndriySvyryd
Copy link
Member

This should throw, but we have a bug in model validation and it doesn't currently check filters.

To configure two indexes give them unique database names:

modelBuilder.Entity<ImplementClassOne>().HasIndex(one => one.UniqueVal)
    .HasDatabaseName("IX_One")
    .IsUnique()
    .HasFilter("Type = 'one'");

modelBuilder.Entity<ImplementClassTwo>().HasIndex(two => two.UniqueVal)
    .HasDatabaseName("IX_Two")
    .IsUnique()
    .HasFilter("Type = 'two'");

In some cases, like when you'd need an index on the base type and a separate one on the derived type you need to instead give them unique model names:

modelBuilder.Entity<Base>().HasIndex(@base => @base.UniqueVal, "IX_Base")
    .IsUnique()
    .HasFilter("Type = 'base'");

modelBuilder.Entity<ImplementClassTwo>().HasIndex(two => two.UniqueVal, "IX_Two")
    .IsUnique()
    .HasFilter("Type = 'two'");

@AndriySvyryd AndriySvyryd changed the title Table-Per-Hierarchy with More than One Filtered Unique Indexes Are Only Applying to First Index Validate that filters are the same for shared indexes May 25, 2021
@AndriySvyryd AndriySvyryd self-assigned this May 25, 2021
@AndriySvyryd AndriySvyryd added this to the 6.0.0 milestone May 25, 2021
@roji roji assigned roji and unassigned AndriySvyryd Sep 8, 2021
@roji roji removed the poachable label Sep 8, 2021
@roji
Copy link
Member

roji commented Sep 8, 2021

Poaching

roji added a commit that referenced this issue Sep 8, 2021
@roji roji 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
roji added a commit that referenced this issue Sep 8, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-rc2 Sep 8, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc2, 6.0.0 Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants