Query: NullReferenceException when property isn't marked as nullable but there is null data

[FransBouma]

Steps to reproduce

On adventureworks, SalesOrderHeader.SalesPersonID, don't mark it as nullable, so give it as type: int

use simple fetch

[Test]
public void LoadSoHs()
{
    using(var ctx = new AdventureWorksDataContext())
    {
        var allRows = (from s in ctx.SalesOrderHeaders  select s).ToList();
        Assert.IsTrue(allRows.Count > 0);
    }
}

NRE

The issue

Query above gives:

System.NullReferenceException : Object reference not set to an instance of an object.
   at lambda_method(Closure , ValueBuffer )
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.UnbufferedEntityShaper`1.Shape(QueryContext queryContext, ValueBuffer valueBuffer)
   at Microsoft.EntityFrameworkCore.Query.QueryMethodProvider.<_ShapedQuery>d__3`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__15`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at AWTests.ReadTests.LoadSoHs() in C:\Temp\generatortest\Test1\AWTests\ReadTests.cs:line 47

Full log:

System.NullReferenceException : Object reference not set to an instance of an object.
   at lambda_method(Closure , ValueBuffer )
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.UnbufferedEntityShaper`1.Shape(QueryContext queryContext, ValueBuffer valueBuffer)
   at Microsoft.EntityFrameworkCore.Query.QueryMethodProvider.<_ShapedQuery>d__3`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__15`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at AWTests.ReadTests.LoadSoHs() in C:\Temp\generatortest\Test1\AWTests\ReadTests.cs:line 47

Compiling query model: 'value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[AW.EntityClasses.SalesOrderHeader])'
Optimized query model: 'value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[AW.EntityClasses.SalesOrderHeader])'
TRACKED: True
(QueryContext queryContext) => IEnumerable<SalesOrderHeader> _ShapedQuery(
    queryContext: queryContext, 
    shaperCommandContext: SelectExpression: 
        SELECT [s].[SalesOrderID], [s].[AccountNumber], [s].[BillToAddressID], [s].[Comment], [s].[ContactID], [s].[CreditCardApprovalCode], [s].[CreditCardID], [s].[CurrencyRateID], [s].[CustomerID], [s].[DueDate], [s].[Freight], [s].[ModifiedDate], [s].[OnlineOrderFlag], [s].[OrderDate], [s].[PurchaseOrderNumber], [s].[RevisionNumber], [s].[rowguid], [s].[SalesOrderNumber], [s].[SalesPersonID], [s].[ShipDate], [s].[ShipMethodID], [s].[ShipToAddressID], [s].[Status], [s].[SubTotal], [s].[TaxAmt], [s].[TerritoryID], [s].[TotalDue]
        FROM [Sales].[SalesOrderHeader] AS [s]
    , 
    shaper: UnbufferedEntityShaper<SalesOrderHeader>
)

Opening connection to database 'AdventureWorksUnitTests' on server 'thor.sd.local'.
Executed DbCommand (13ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
SELECT [s].[SalesOrderID], [s].[AccountNumber], [s].[BillToAddressID], [s].[Comment], [s].[ContactID], [s].[CreditCardApprovalCode], [s].[CreditCardID], [s].[CurrencyRateID], [s].[CustomerID], [s].[DueDate], [s].[Freight], [s].[ModifiedDate], [s].[OnlineOrderFlag], [s].[OrderDate], [s].[PurchaseOrderNumber], [s].[RevisionNumber], [s].[rowguid], [s].[SalesOrderNumber], [s].[SalesPersonID], [s].[ShipDate], [s].[ShipMethodID], [s].[ShipToAddressID], [s].[Status], [s].[SubTotal], [s].[TaxAmt], [s].[TerritoryID], [s].[TotalDue]
FROM [Sales].[SalesOrderHeader] AS [s]
Closing connection to database 'AdventureWorksUnitTests' on server 'thor.sd.local'.
An exception occurred in the database while iterating the results of a query.
System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method(Closure , ValueBuffer )
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.UnbufferedEntityShaper`1.Shape(QueryContext queryContext, ValueBuffer valueBuffer)
   at Microsoft.EntityFrameworkCore.Query.QueryMethodProvider.<_ShapedQuery>d__3`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__15`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()

This isn't really helpful:

1. the exception shouldn't happen at all, it should result to default(type). This is because a resultset with nulls due to left joins projected to a POCO with fields which aren't nulls (the user might not know) shouldn't crash. One could actually get away by not setting it at all, simply skip setting the field if the value is null and the target type is a non nullable valuetype.
2. the NRE doesn't say me anything. I luckily had a class to compare it with and found the nullable property after half an hour, but otherwise it would have been impossible to find.

Further technical details

EF Core version: 1.0.1, running on .NET full (4.5.2)
Operating system: windows 8.1
Visual Studio version: 2015

[FransBouma]

It also shows up when the fields in the projection are properly defined: https://gist.github.com/FransBouma/5d92ddcaa185a9f613b1f50081b28862 There, the SalesOrderHeader type is part of a projection from which the final projection is created, but it shouldn't be used for materialization, it will never happen. Or at least, it shouldn't. So the modeling mistake on SalesOrderHeader.SalesPersonID shouldn't be causing this query to crash but it does (with the same stacktrace).

It's weird though, correcting the SalesPersonID mapping to make it nullable, should make the query in the Gist work, but it doesn't. All mappings should be OK now, no errors in your model checker nor in mine. Same exception.

(additionally random note: you really should tell your users that DefaultIfEmpty is run in memory, normal users who don't enable full logging won't be aware of that and will pull lots of data in memory)

[FransBouma]

@rowanmiller What does '1.2.0' mean btw? The query with the joins in the gist still crashes. I have no idea why (as in: is it my fault in the mappings or a bug in EF?) Will people have to wait till 1.2.0 hits the streets (which is in 2017 somewhere) ? What should I tell my customers who will use my designer, generate queries from their model, code which compiles fine and is accepted by your validator as OK, but crashes at runtime? That they have to wait till 1.2.0?

[rowanmiller]

The issue is marked for investigation, which means we want to dig into it a bit more. 1.1 is pretty much wrapped up, so assignment in 1.2 means it is on our "current list of work".

We think that the NullReferenceException is fixed in 1.1. You should now get a better exception saying that we got a null from the database but we couldn't set it to the property because it didn't support nulls. Verifying this would be the first part of our investigation.

One thing to clarify though...

The exception shouldn't happen at all, it should result to default(type).

This isn't how we have designed things. If we can't populate the property with the value from the database, then we will throw. If you have nulls in the database then you need to use a property type that is capable of holding them.

[FransBouma]

1.1 is pretty much wrapped up, so assignment in 1.2 means it is on our "current list of work".

Ok. I'll mark it in our docs as a warning for users, that generated queries by our designer might crash at runtime still, and refer to this issue. The join query shouldn't throw the null ref, as everything is properly defined for nulls.

This isn't how we have designed things. If we can't populate the property with the value from the database, then we will throw. If you have nulls in the database then you need to use a property type that is capable of holding them.

But what if I have a left joined set and project it to a poco? Didn't EF6 use default(type) ? The thing is that the developer might test it with proper data, things work fine, but in practice in production things might crash with a null ref exception because due to a left join a null came back from the DB. It's up to you of course, but crashing out is IMHO not in the interest of the user, default(type) is.

[maumar]

in case of left joins (e.g. when using optional navigation) we add null-compensation.

Example:

from c in customers
select c.Address.Zip;

(assuming there can be a customer without an address) will get translated to:

from c in customers
join a in addresses on c.Id equal a.CustomerId into agroup
from a in agroup.DefaultIfEmpty()
select a != null ? a.Zip : null

However, if the left-joined type is non-nullable (say, int) users need to help us with the possible null, either by casting the result to (int?) or using ?? operator:

from c in customers
select (int?)c.Address.HomeNumber; // or select c.Address.HomeNumber ?? 0

[FransBouma]

I did some tests, my framework silently accepts it, EF6 also throws an exception about the non-nullability of an int (in case of left-join between customer and order in NW). IMHO, it's a burden to users, but it's your framework (and it doesn't have to cost any performance either) ;) It's a bit of a grey area, but anyway, thanks for clearing that up.

I'll leave it open as the issue of the joined set is still unclear.

[maumar]

NRE is indeed gone, now we tell the users that expected value was int, but got null instead.