Mapping identity columns to enum properties no longer works in 2.1

[chrarnoldus]

In EF Core 2.0 it is possible to map an identity column in SQL Server to an enum property in C#. In EF Core 2.1 this no longer works and an exception is thrown (see details below). The reason we want use enum types rather than ints is to prevent mixing up identifiers of different entities.

Steps to reproduce

Code to reproduce the issue: https://github.com/chrarnoldus/EnumValueGeneratedOnAdd

Leaving the version on 2.0.3 and running the code, the programs prints the generated ids of the entities that were added, which is what I expect:

1
2
3
4
5
6
7
8
10

When changing the version to 2.1.0-rc1-final and running again, it crashes with the following exception:

Unhandled Exception: System.NotSupportedException: Value generation is not supported for property 'Entity.EntityId' because it has a 'EnumToNumberConverter<EntityId, int>' converter configured. Configure the property to not use value generation using 'ValueGenerated.Never' or 'DatabaseGeneratedOption.None' and specify explict values instead.
   at Microsoft.EntityFrameworkCore.ValueGeneration.ValueGeneratorSelector.CreateFromFactory(IProperty property, IEntityType entityType)
   at Microsoft.EntityFrameworkCore.ValueGeneration.ValueGeneratorSelector.<Select>b__6_0(IProperty p, IEntityType t)
   at Microsoft.EntityFrameworkCore.ValueGeneration.ValueGeneratorCache.<>c.<GetOrAdd>b__3_0(CacheKey ck)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.ValueGeneration.ValueGeneratorCache.GetOrAdd(IProperty property, IEntityType entityType, Func`3 factory)
   at Microsoft.EntityFrameworkCore.SqlServer.ValueGeneration.Internal.SqlServerValueGeneratorSelector.Select(IProperty property, IEntityType entityType)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ValueGenerationManager.Generate(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode node, Boolean force)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode node, TState state, Func`3 handleNode)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState[TEntity](TEntity entity, EntityState entityState)
   at EnumValueGeneratedOnAdd.Program.Main(String[] args) in C:\Users\Christiaan\Documents\Visual Studio 2017\Projects\EnumValueGeneratedOnAdd\EnumValueGeneratedOnAdd\Program.cs:line 45

Is this something you would consider fixing? If not, is there another way to make this work?

Further technical details

EF Core version: 2.1.0-rc1-final
Database Provider: Microsoft.EntityFrameworkCore.SqlServer (SQL Server version doesn't seem to matter, tried 2016 LocalDB and 2017 Developer)
Operating system: Windows 10 1803
IDE: Visual Studio 2017 15.7.1

[ajcvickers]

@chrarnoldus This is an interesting pattern that I hadn't considered before--it's like you're making C# into ADA! The thing that changed here is that enums previously had ad-hoc conversions, but now are just a case of more general value converters. This means #11597 is now being hit in this case. We will consider this when working on that issue, but also leaving this open as a related bug since enums with no defined set of values may end up being a special case.

I was able to workaround this by specifying my own converter and value generator:

var converter = new ValueConverter<EntityId, int>(
    v => (int)v,
    v => (EntityId)v,
    new ConverterMappingHints(valueGeneratorFactory: (p, t) => new TemporaryIntValueGenerator()));

modelBuilder
    .Entity<Entity>()
    .Property(p => p.EntityId)
    .HasConversion(converter);

(I actually didn't think this would work, but it seems to. If you use this approach and run into any issues then please let us know.)

[TomCJones]

i get the same error for this following, is only int support, not long or ulong, that seems odd
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public UInt64 id { get; set; }

[space-alien]

@roji Just wondering if this could be a factor - from the exception message: Identity value generation can only be used with signed integer properties. Maybe SQL Server only permits signed ints for PKs? Whereas Postgre supports short, int and long, and in this example we've used the latter.

[chrarnoldus]

We've used that trick for a while, but it stopped working with 3.0.

[roji]

@space-alien SQL Server supports IDENTITY on bigint just fine. I've removed the long in my sample above to simplify that.

[ajcvickers]

Moving to backlog since this is dependent on #11597

[NickNiebling]

I would love to see this feature coming back to EF Core 5.0.

I'm no expert in how to implement this the best way, but I quickly came up with 2 approaches that might be viable:

1. Adding an "IntValueConverter" that always convert to Integer, and allowing this converter be used for Identity value generation
2. Specifically allowing Enums to be converted to their underlying Integral numeric (Int, long, ...) when used in Models

The first option might provide more freedom to the developer (maybe having their own type that can be converted to/from int might help some?), while it might require more effort for each developer to "simply" convert between enum and int (creating custom converter and making use of it potentially for "all primary keys").

The second option might provide out of the box support (as in the past?), making it "easy" for anyone to opt in for Enum instead of e.g. Int. This is my preferred choice, but again: I'm not sure why support was removed in the first place, and there might be good reasons for this that makes this approach bad.

Just a general note: Preferring enums over ints has really been helpful for me for several years and it feels like it will continue to be. Especially having a method with args like (int, int) can be annoying compared to (Enum1, Enum2).

[reinux]

I'd like this feature too, though not for enums.

I use an int value converter for all my ID types, and the IDs are wrapped in records to improve type safety:

  public abstract record IntId
  {
    public int Id { get; private init; }
    public IntId(int id) => Id = id;
    public static object Generate(Type type, int id) =>
      type.GetConstructor(new[] {typeof(int)}).Invoke(new object[] {id});
  }
  
  public record AccountId(int Id) : IntId(Id);
  public record CompanyId(int Id) : IntId(Id);
  public record UserId(int Id) : IntId(Id);
  ...

This pattern is a very effective way to avoid a common class of bugs having to do with incorrect ID assignments, and provides a lot of documentation value.

[lonix1]

@reinux What you're describing is a DDD Value Object, see my related issue here. Did you find a solution by upgrading to EF Core 6, or find some other workaround?