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

Set MaxLength on TPH discriminator property by convention #10691

Closed
satyajit-behera opened this issue Jan 12, 2018 · 6 comments · Fixed by #29946
Closed

Set MaxLength on TPH discriminator property by convention #10691

satyajit-behera opened this issue Jan 12, 2018 · 6 comments · Fixed by #29946
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-enhancement
Milestone

Comments

@satyajit-behera
Copy link

The Discriminator column in TPH is created nvarchar(max) by default.

Can we set it to a fixed length (say nvarchar(200)) and index it globally with one setting.

Thanks

@ajcvickers
Copy link
Contributor

@satyajit-behera I just sent a PR to update the docs for it: dotnet/EntityFramework.Docs#601. Here's the update:

Configuring the discriminator property

In the examples above, the discriminator is created as a shadow property on the base entity of the hierarchy. Since it is a property in the model, it can be configured just like other properties. For example, to set the max length when the default, by-convention discriminator is being used:

modelBuilder.Entity<Blog>()
    .Property("Discriminator")
    .HasMaxLength(200);

The discriminator can also be mapped to an actual CLR property in your entity. For example:

class MyContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Blog>()
            .HasDiscriminator<string>("BlogType");
    }
}

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

public class RssBlog : Blog
{
    public string RssUrl { get; set; }
}

Combining these two things together it is possible to both map the discriminator to a real property and configure it:

modelBuilder.Entity<Blog>(b =>
{
    b.HasDiscriminator<string>("BlogType");

    b.Property(e => e.BlogType)
        .HasMaxLength(200)
        .HasColumnName("blog_type");
});

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Jan 12, 2018
@satyajit-behera
Copy link
Author

Thanks. This fixes for each entity individually. Is there any way where I can set it for all the discriminators across Context in one setting.

@ajcvickers
Copy link
Contributor

@satyajit-behera Run through all the entity types at the end of OnModelCreating and add configuration. For example, something like this:

foreach (var entityType in modelBuilder.Model.GetEntityTypes())
{
    entityType.FindProperty("Discriminator")?.SetMaxLength(200);
}

@satyajit-behera
Copy link
Author

@ajcvickers Thanks a lot

@Schuttrim
Copy link

I couldn't set the max Length using the b.Property(e => e.BlogType).HasMaxLength(200) syntax, ince I'm using the ToTable(string) Method which isn't available after using the Entity<TEntity>([NotNull] Action<EntityTypeBuilder<TEntity>> buildAction) Method, since it returns a ModelBuilder instead of EntityTypeBuilder.

Thats why I added the configuration right after with the following code:

modelBuilder.Model.GetEntityTypes(typeof(Blog)).Single().FindProperty("BlogType").SetMaxLength(200);

@ajcvickers ajcvickers reopened this Aug 24, 2022
@ajcvickers ajcvickers self-assigned this Aug 31, 2022
@ajcvickers ajcvickers added area-model-building consider-for-next-release and removed closed-no-further-action The issue is closed and no further action is planned. labels Aug 31, 2022
@ajcvickers ajcvickers added this to the Backlog milestone Aug 31, 2022
@ajcvickers ajcvickers changed the title Setting Field Size for Discriminator column in TPH Set MaxLength on TPH discriminator property by convention Aug 31, 2022
@ajcvickers
Copy link
Contributor

Note from triage: consider using an approach like shown here: dotnet/EntityFramework.Docs#4000, possibly using power-of-two buckets to avoid a migration on every change to discriminator value length.

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 29, 2022
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0 Dec 29, 2022
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Jan 29, 2023
@ajcvickers ajcvickers removed this from the 8.0.0-preview1 milestone Nov 14, 2023
@ajcvickers ajcvickers added this to the 8.0.0 milestone Nov 14, 2023
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
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-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants