Exception when loading entity containing a primitive property with matching System.Nullable backing field

[KSid]

Using EF Core 2.1 throws an exception when loading entities that have mapped properties with primitive types that have matching System.Nullable backing fields of the same type (e.g. int property with int? backing field). This has been working without problem on previous releases of EF Core (1.1.x - 2.0.x).

P.S. Is there an alternative approach available that avoids hardcoding the name of the backing field during model mapping? It doesn't look like the new conversion facilities will work here due to the (current) inability to convert null.

Exception message:
Expression of type 'System.Int32' cannot be used for assignment to type 'System.Nullable`1[System.Int32]'

Stack trace:
at System.Linq.Expressions.Expression.Assign(Expression left, Expression right)
at System.Linq.Expressions.Expression.MakeBinary(ExpressionType binaryType, Expression left, Expression right, Boolean liftToNull, MethodInfo method, LambdaExpression conversion)
at System.Linq.Expressions.BinaryExpression.Update(Expression left, LambdaExpression conversion, Expression right)
at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
at System.Linq.Expressions.BlockExpression.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.CreateExecutorLambda[TResults]()
at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateAsyncQueryExecutor[TResult](QueryModel queryModel)
at Microsoft.EntityFrameworkCore.Storage.Database.CompileAsyncQuery[TResult](QueryModel queryModel)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileAsyncQueryCore[TResult](Expression query, IQueryModelGenerator queryModelGenerator, IDatabase database)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass22_0`1.<CompileAsyncQuery>b__0()
at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddAsyncQuery[TResult](Object cacheKey, Func`1 compiler)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileAsyncQuery[TResult](Expression query)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query)
at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression)
at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.System.Collections.Generic.IAsyncEnumerable<TResult>.GetEnumerator()
at System.Linq.AsyncEnumerable.<Aggregate_>d__6`3.MoveNext()--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at UserQuery.<Main>d__0.MoveNext()
in C:\Users\Kawsar\AppData\Local\Temp\LINQPad5\_rpxfsumo\query_cvmzib.cs:line 49

Steps to reproduce

async Task Main()
{
    using (var db = new AppDbContext(creatingDatabase: true))
    {
        await db.Database.EnsureDeletedAsync();
        await db.Database.EnsureCreatedAsync();

        db.Orders.Add(new Order(null));
        await db.SaveChangesAsync();
    }

    using (var db = new AppDbContext(creatingDatabase: false))
    {
        // N.B. contrived example, can be simplified to
        // var order = await db.Orders.FirstOrDefaultAsync();
        // which does not produce the same exception
        var order = (await db.Orders.ToListAsync()).FirstOrDefault();

        // a foreach loop also generates the same exception
        // foreach (var order in db.Orders) {}

        switch (order?.NullableInt)
        {
            case -1:
                Console.WriteLine($"Success! Loaded expected order: NullableInt is {order.NullableInt}");
                break;
            case null:
                Console.WriteLine("FAILED! Could not find an order");
                break;
            default:
                Console.WriteLine($"FAILED! Loaded unexpected order: NullableInt is {order.NullableInt}");
                break;
        }
    }
}

sealed class Order
{
    public int Id { get; private set; }

    private int? _nullableInt;
    public int NullableInt => _nullableInt ?? -1;

    private Order()
    {

    }

    public Order(int? nullableInt)
    {
        _nullableInt = nullableInt;
    }
}

class AppDbContext : DbContext
{
    public AppDbContext(bool creatingDatabase)
    {
        CreatingDatabase = creatingDatabase;
    }

    public bool CreatingDatabase { get; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        base.OnConfiguring(optionsBuilder);

        optionsBuilder
            .UseSqlServer("Data Source=(localdb)\\MSSQLLocalDB;Initial Catalog=Repro12215;Trusted_Connection=True;MultipleActiveResultSets=true;")
            .UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking)
            .ReplaceService<IModelCacheKeyFactory, DynamicModelCacheKeyFactory>();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        if (CreatingDatabase)
        {
            // used to correctly create required database field for repro
            modelBuilder.Entity<Order>()
                .Property("_nullableInt")
                .HasColumnName("NullableInt")
                .IsRequired(false);
        }
        else
        {
            // actual runtime configuration used in app with EF Core 1.1.x - 2.0.x
            modelBuilder.Entity<Order>()
                .Property(m => m.NullableInt);
        }
    }
    
    public DbSet<Order> Orders => Set<Order>();
}

public class DynamicModelCacheKeyFactory : IModelCacheKeyFactory
{
    public object Create(DbContext context)
    {
        if (context is AppDbContext appDbContext)
        {
            return (context.GetType(), appDbContext.CreatingDatabase);
        }
        return context.GetType();
    }
}

Further technical details

EF Core version: 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 Version 1803
IDE: Visual Studio 2017 15.7.3

[ajcvickers]

@KSid It's surprising that this worked in 2.0. Having a property that returns a different value than it was set to will often result in issues. For example, using this pattern, how do you ensure that -1 is not saved to the database?

That being said, this does look like a bug generating the query.

[KSid]

Thanks for taking a look, @ajcvickers. I have done a little more digging and found the exception is only thrown when using QueryTrackingBehavior.NoTracking and affects the new 'query types' feature regardless of this setting. It can be also reproduced without involving the database as the exception is thrown before EF attempts a connection.

Fortunately the affected class is readonly and interfaces with a third party database so I've not needed to test how it behaves when updating the field and saving the changes. If it became a requirement and we wanted to keep storing nulls instead of -1, I suspect we would have to hard code the backing fields as magic strings in the model mapping or use a leaky model by exposing the int? directly alongside an additional unmapped property just to be used by the application code. A cleaner, less magical option would probably be to take advantage of the new conversion features if they are updated in the future to support primitive to primitive? conversions.

[KSid]

Thank you for assigning some time to this, @ajcvickers. Is the 2.2 milestone an estimate? I was hoping the regression could be fixed and released as part of a 2.1.x release.

[ajcvickers]

@KSid There is some possibility this could get patched, but right now it's just above the patch bar. Things that could change this are:

• Demonstration that this is an issue when the value of the property read reflects the value of the property set. (Essentially, what I am saying here is that for properties that are not well-behaved, it is possible for many undefined behaviors to happen, and hence even if we fix this issue there very well may be others that it is masking because the application code is not behaving in a generally acceptable manner.)
• Reports from multiple people hitting the same issue.

[smitpatel]

Error in 3.1

Unhandled exception. System.Data.SqlTypes.SqlNullValueException: Data is Null. This method or property cannot be called on Null values.
   at Microsoft.Data.SqlClient.SqlBuffer.ThrowIfNull()
   at Microsoft.Data.SqlClient.SqlBuffer.get_Int32()
   at Microsoft.Data.SqlClient.SqlDataReader.GetInt32(Int32 i)
   at lambda_method(Closure , QueryContext , DbDataReader , ResultContext , Int32[] , ResultCoordinator )
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at EFSampleApp.Program.Main() in D:\code\EFSampleApp\EFSampleApp\Program.cs:line 29
   at EFSampleApp.Program.<Main>()

@ajcvickers - Can you give this a read and see if this is expected?

[ajcvickers]

@smitpatel I still wouldn't expect this to work, and the exception is certainly linked to the way the property is mapped. Will look at this while working on #15182.

[ajcvickers]

Took another look at this in relation to #15182. That change allows a nullable backing field to be used for a non-nullable property. However, there are two additional behaviors here. The first is to allow the column to be created in the database as nullable, even though it is non-nullable in C-space. This is can be done by editing the generated migration, although its's a strange thing to do, since nulls from the database would not be able to be used. However, the second behavior is an apparent desire to replace nulls with -1, at least when null is supplied by the client, and possibly when it comes from the database. This then falls under the value conversions to and from null, tracked by #13850.