dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.49k stars 3.13k forks source link

Result of left joining a nullable nominal type should be null (i.e. no instance) and not an instance with all null property values #22517

Open skclusive opened 3 years ago

skclusive commented 3 years ago

when Blog does not have a Post, following query does not work in 5.0.0-preview.8. or 6.0.0- nightly. but works in 5.0.0-preview.7.*


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

        [NotMapped]
        public Post Post { get; set; }

        public List<Post> Posts { get; set; } = new List<Post>();
    }

    public class Post
    {
        public int PostId { get; set; }

        public string Title { get; set; }

        public string Content { get; set; }

        public int BlogId { get; set; }

        public Blog Blog { get; set; }
    }

// this IQueryable would come from other API.

 var dbPosts = from p in db.Posts
                          // select p;
                         select new Post
                         {
                             PostId = p.PostId,
                             BlogId = p.BlogId,
                             Content = p.Content
                         };

 var query = from blog in db.Blogs
                    join post in dbPosts on blog.BlogId equals post.BlogId into posts
                    from xpost in posts.DefaultIfEmpty()
                    select new Blog
                    {
                         Url = blog.Url,
                         Post = xpost
                   };

Steps to reproduce

I have a repo to reproduce the bug.

https://github.com/skclusive/EFLeftJoinBug

Unhandled exception. System.InvalidOperationException: Nullable object must have a value.
   at lambda_method17(Closure , QueryContext , DbDataReader , ResultContext , SingleQueryResultCoordinator )
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()

Further technical details

EF Core version: Database provider: (e.g. Microsoft.EntityFrameworkCore.Sqlite) Target framework: (e.g. .NET Core 5.0) Operating system: IDE: (e.g. Visual Studio Code)

ajcvickers commented 3 years ago

Confirmed this works in 3.1, but fails in latest daily, on SQL Server and SQLite.

smitpatel commented 3 years ago

@ajcvickers - What is the generated SQL? Can you post query plan?

ajcvickers commented 3 years ago

@smitpatel Here's the logs:

dbug: Microsoft.EntityFrameworkCore.Query[10111]
      Compiling query expression: 
      'DbSet<Blog>()
          .GroupJoin(
              inner: DbSet<Post>()
                  .Select(p => new Post{ 
                      PostId = p.PostId, 
                      BlogId = p.BlogId, 
                      Content = p.Content 
                  }
                  ), 
              outerKeySelector: blog => blog.BlogId, 
              innerKeySelector: post => post.BlogId, 
              resultSelector: (blog, posts) => new { 
                  blog = blog, 
                  posts = posts
               })
          .SelectMany(
              collectionSelector: <>h__TransparentIdentifier0 => <>h__TransparentIdentifier0.posts
                  .DefaultIfEmpty(), 
              resultSelector: (<>h__TransparentIdentifier0, xpost) => new Blog{ 
                  Url = <>h__TransparentIdentifier0.blog.Url, 
                  Post = xpost 
              }
          )'
dbug: Microsoft.EntityFrameworkCore.Query[10107]
      queryContext => new SingleQueryingEnumerable<Blog>(
          (RelationalQueryContext)queryContext, 
          RelationalCommandCache.SelectExpression(
              Projection Mapping:
                  Url -> 0
                  Post.PostId -> 1
                  Post.BlogId -> 2
                  Post.Content -> 3
              SELECT b.Url, p.PostId, p.BlogId, p.Content
              FROM Blogs AS b
              LEFT JOIN Posts AS p ON b.BlogId == p.BlogId), 
          Func<QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator, Blog>, 
          EFLeftJoinBug.BloggingContext, 
          False, 
          False
      )
dbug: Microsoft.EntityFrameworkCore.Database.Command[20103]
      Creating DbCommand for 'ExecuteReader'.
dbug: Microsoft.EntityFrameworkCore.Database.Command[20104]
      Created DbCommand for 'ExecuteReader' (0ms).
dbug: Microsoft.EntityFrameworkCore.Database.Connection[20000]
      Opening connection to database 'Test' on server '(local)'.
dbug: Microsoft.EntityFrameworkCore.Database.Connection[20001]
      Opened connection to database 'Test' on server '(local)'.
dbug: Microsoft.EntityFrameworkCore.Database.Command[20100]
      Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [b].[Url], [p].[PostId], [p].[BlogId], [p].[Content]
      FROM [Blogs] AS [b]
      LEFT JOIN [Posts] AS [p] ON [b].[BlogId] = [p].[BlogId]
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (7ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [b].[Url], [p].[PostId], [p].[BlogId], [p].[Content]
      FROM [Blogs] AS [b]
      LEFT JOIN [Posts] AS [p] ON [b].[BlogId] = [p].[BlogId]
fail: Microsoft.EntityFrameworkCore.Query[10100]
      An exception occurred while iterating over the results of a query for context type 'EFLeftJoinBug.BloggingContext'.
      System.InvalidOperationException: Nullable object must have a value.
         at lambda_method(Closure , QueryContext , DbDataReader , ResultContext , SingleQueryResultCoordinator )
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
System.InvalidOperationException: Nullable object must have a value.
   at lambda_method(Closure , QueryContext , DbDataReader , ResultContext , SingleQueryResultCoordinator )
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
Unhandled exception. System.InvalidOperationException: Nullable object must have a value.
   at lambda_method(Closure , QueryContext , DbDataReader , ResultContext , SingleQueryResultCoordinator )
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
   at EFLeftJoinBug.Program.Main(String[] args) in /home/ajcvickers/AllTogetherNow/Daily/Daily.cs:line 43
smitpatel commented 3 years ago

SingleQueryResultCoordinator is not the error. We are probably trying to materialize null into something which is non-nullable which should have been skipped from materialization.

smitpatel commented 3 years ago

Note: This gave incorrect result in 3.1.x by initialize post with default values rather than returning null (the result of DefaultIfEmpty).

With #20633 fixed, it now throws error.

Right fix would be not to create instance if no rows matched.

smitpatel commented 3 years ago

Issues faced: When generating left join we need to add additional null check in selector so that we can generate correct default type if sequence is empty and not try to materialize result.

Due to unintended side effect of breaking working queries due to having a complex expression for entity now, we should postpone this.

smitpatel commented 3 years ago

Work-around to original issue

 var query = from blog in db.Blogs
                    join post in dbPosts on blog.BlogId equals post.BlogId into posts
                    from xpost in posts.DefaultIfEmpty()
                    select new Blog
                    {
                         Url = blog.Url,
                         Post = xpost == null ? null : xpost
                   };
skclusive commented 3 years ago

i did try the following work-around before raising the bug. this workaround doesn't work :(

 var query = from blog in db.Blogs
                    join post in dbPosts on blog.BlogId equals post.BlogId into posts
                    from xpost in posts.DefaultIfEmpty()
                    select new Blog
                    {
                         Url = blog.Url,
                         Post = xpost == null ? null : xpost
                   };
ajcvickers commented 3 years ago

@skclusive Did you try the workaround using EF Core 5.0 RC1 or EF Core 3.1.x?

skclusive commented 3 years ago

@ajcvickers i tested with 6.0.0-* nightly. now tried with EF Core RC1 also. same issue.

skclusive commented 3 years ago

i guess this is critical issue.

if i use in memory objects. following does work with standard linq query. so i am wondering why #20633 considered bug.

var memoryBlogs = new List<Blog>
                {
                    new Blog
                    {
                        BlogId = 1,
                        Url = "http://blogs.msdn.com/adonet"
                     }
                };

                var memoryPosts = from p in new List<Post>()
                            select new Post
                            {
                                PostId = p.PostId,
                                BlogId = p.BlogId,
                                Content = p.Content
                            };

                var query = from blog in memoryBlogs
                            join post in memoryPosts on blog.BlogId equals post.BlogId into posts
                            from xpost in posts.DefaultIfEmpty()
                            select new Blog
                            {
                                Url = blog.Url,
                                Post = xpost
                            };

when there is no posts, following does not throw error in the projection. so why throwing error when composed.


var posts = from p in db.Posts
                            select new Post
                            {
                                PostId = p.PostId, // we dont consider p null here
                                BlogId = p.BlogId,
                                Content = p.Content
                            };

                var post = posts.SingleOrDefault(); // query is success with null value. no error on projection.

                Console.WriteLine(post?.PostId);

i am composing IQueryable extensively in my code and this recent change breaks my framework. so please consider to address this issue.

smitpatel commented 3 years ago

@skclusive - Your query was giving incorrect results in 3.1, The bug fix for #20633 stopped generating incorrect results. In your case it throws exception because the query translation couldn't figure out a way to generate right results. Your query working in LINQ is irrelevant as it worked in 3.1 also and EF Core generated results different than LINQ.

                var dbPosts = from p in db.Posts
                                  // select p;
                              select new Post
                              {
                                  PostId = p.PostId,
                                  BlogId = p.BlogId,
                                  Content = p.Content
                              };

                var query = from blog in db.Blogs
                            join post in dbPosts on blog.BlogId equals post.BlogId into posts
                            from xpost in posts.DefaultIfEmpty()
                            select new Blog
                            {
                                Url = blog.Url,
                                Post = xpost.BlogId == null ? null : xpost
                            };

Tested that above work-around gives correct result on 5.0 rc2 nightly build.

skclusive commented 3 years ago

@smitpatel got it. thanks.

the above mentioned workaround does work in nightly and rc-1.

only issue is this comparison produce warning xpost.BlogId == null (comparing int to null). Also will this be documented as i guess some might get this issue frequently.

being workaround will this be addressed in future releases?

you can close the issue if no further action involved.

shadowfoxish commented 3 years ago

https://stackoverflow.com/a/65207398/1181624

You can also do a cast to a nullable type to make this issue go away. I posted that stack overflow answer before I found this post.

select new {
   ...
  optionalValue = (int?)leftJoinedType.someInt
}
hhhuut commented 3 years ago

Still happens with MetadataGenerator v1.2.0 and the latest EFC packages (5.0.1).

However from what I understand, this is now the expected behaviour and we'll have to add explicit "workarounds" to all left-joined queries like the one @shadowfoxish mentioned?

smitpatel commented 3 years ago

@hhhuut - There is no one answer to the question. Essentially, when you do a left join, right side can have null values so accessing any property over that will give you back null (EF Core will make property access null safe). But in C# there is no concept of left join or null-safe access. So in C# the property access will take CLR type of the property which can/cannot accommodate null value. e.g. entity.Property where entity is coming from right side of left join and Property is of type int will return int value. If entity turns out to be null then it throws null-ref exception. Since EF Core makes the access null safe it is same as writing entity == null ? (int?)null : entity.Property which will make the return value of type int? So when actual null value is encountered then it cannot be put into a int value throwing above exception. This only happens at runtime, when you actually encounter a null value.

So, as a user, you should carefully evaluate queries and see if you are expecting a null value appearing in left join and the property-accesses are able to accommodate that null value by being nullable type. In such cases you are required to put nullable type cast. It is not a "work-around", it is required to materialize the results from the data you have. Though if your query will never get null from left join (be careful as data can change over the type in an app), or you are not accessing a property on an entity which can be nullable due to left join then you shouldn't need it. The same exception can also arise when database contains null but client side property cannot take null values (even without left-joins) due to mismatch in model. There is also a possibility that there is a bug in EF Core somewhere. So if you have a query which has result types correct based on expected data and seeing this exception please file a new issue.

cgountanis commented 3 years ago

Just seems like a pain if you have a table with optional FK. When you use .Select() to optimize it just blows up with "Nullable object must have a value."

return _context.Something
    .OrderByDescending(j => j.X)
    .Include(r => r.X)
    .Include(r => r.XX)

    .Select(r => new Something()
    {
        X1 = r.X1,
        X2 = r.X2,

        X = new X() 
        {
            Name= r.X.Name
        },
        XX = r.XXNullableId == null ? null : new XX()
        {
            XNumber = r.XX.XNumber
        }
    })
    .AsSingleQuery()
    .AsNoTracking();

You end up getting this strange CASE statement but it seems to work fine this way.

CASE WHEN [d].[XXId] IS NULL THEN CAST(1 AS bit) ELSE CAST(0 AS bit)

Is this the best approach?

jayeshdshah commented 3 years ago

@hhhuut - There is no one answer to the question. Essentially, when you do a left join, right side can have null values so accessing any property over that will give you back null (EF Core will make property access null safe). But in C# there is no concept of left join or null-safe access. So in C# the property access will take CLR type of the property which can/cannot accommodate null value. e.g. entity.Property where entity is coming from right side of left join and Property is of type int will return int value. If entity turns out to be null then it throws null-ref exception. Since EF Core makes the access null safe it is same as writing entity == null ? (int?)null : entity.Property which will make the return value of type int? So when actual null value is encountered then it cannot be put into a int value throwing above exception. This only happens at runtime, when you actually encounter a null value.

So, as a user, you should carefully evaluate queries and see if you are expecting a null value appearing in left join and the property-accesses are able to accommodate that null value by being nullable type. In such cases you are required to put nullable type cast. It is not a "work-around", it is required to materialize the results from the data you have. Though if your query will never get null from left join (be careful as data can change over the type in an app), or you are not accessing a property on an entity which can be nullable due to left join then you shouldn't need it. The same exception can also arise when database contains null but client side property cannot take null values (even without left-joins) due to mismatch in model. There is also a possibility that there is a bug in EF Core somewhere. So if you have a query which has result types correct based on expected data and seeing this exception please file a new issue.

Thank you @smitpatel for this comment. It helped me in fixing my issue. Below is my left join linq query in which I was getting some null values from right side table(which was expected) which was causing the above error. After reading your comments I added nullable type in rightside table query and null check in joining part of left and right table.

    return await (from items in _dbContext.LeftTable
                      join LeftJoinTable in ((from iss in _dbContext.RightTable
                                                where iss.CId == cId && iss.ActionDate == null
                                                select new { IId = (int?)iss.IId }).Distinct())
                      on items.Id equals (LeftJoinTable.IId ?? 0)  into ljoin
                      from LeftJoinTable in ljoin.DefaultIfEmpty()
                      orderby items.Name
                      select new CIDto
                      {
                          Id = items.Id,
                          Name = items.Name ,
                          IType = items.IType,
                          IsActive = (LeftJoinTable.IId ?? 0) == 0 ? 0 : 1 
                      }).AsNoTracking().ToListAsync();
Mijalski commented 3 years ago

Is there any word on when this issue may get resolved?

ajcvickers commented 3 years ago

@Mijalski This issue is in the 6.0 milestone, which means we plan to fix it for EF Core 6.0.

StrangeWill commented 3 years ago

Still happens with MetadataGenerator v1.2.0 and the latest EFC packages (5.0.1).

However from what I understand, this is now the expected behaviour and we'll have to add explicit "workarounds" to all left-joined queries like the one @shadowfoxish mentioned?

@Mijalski This issue is in the 6.0 milestone, which means we plan to fix it for EF Core 6.0.

Yeah I'm a little confused here on what to expect in EF 6, are we going to expect the original behavior to come back in EF Core 6.0? We have a code base that we could add a ton of conditional checks for 3-4 relationships deep but that would make complete mess of our selectors (if only we could do null conditionals in expression trees...), so we're effectively on .NET 5.0 with EF Core 3.1 (this isn't the first project we had to do this with).

An example of one of our selectors:

  Procedure = new
  {
      cf.Field.Procedure.Name,
      cf.Field.Procedure.FollowUpInterval
  },

Checking cf, field and procedure each time is a bit rough.

Are we expecting EF Core 3.1 code to be compatible with EF 6? (Mainly just determining if I need to start migrating code to the "new" way or just wait around for EF Core 6).

Any possibility we'll see this backported to 5?

smitpatel commented 3 years ago

@StrangeWill - This is what will happen in EF Core 6.0

Finally, this cannot be backported to 5.0 due to complexity of the fix.

optiks commented 3 years ago

We've just hit this after (attempting) to upgrade from 3.1 to 5.0. From what I can tell, this is a significant behavioural change.

Suggestion: Mention this is Breaking changes in EF Core 5.0?

Googling stack traces isn't much fun :)

eric-miller-cdfw commented 2 years ago

It's too bad LINQ expressions don't support the null conditional operator ?. Then we could indicate a possible null in the navigation to the EF query and quiet the VS Analyzers that complain about a possible null dereference (CS8602).

.Select(x => new {
    x.NavProperty1?.NavProperty2?.NonNullValueTypeProperty,
})
Shiko1st commented 2 years ago

This is definitely a breaking change

smitpatel commented 2 years ago

Related #19095

channeladam commented 1 year ago

This is very inconvenient, and has been 'punted' for 2 versions now...

cyril265 commented 1 year ago

So, as a user, you should carefully evaluate queries and see if you are expecting a null value appearing in left join and the property-accesses are able to accommodate that null value by being nullable type. In such cases you are required to put nullable type cast. It is not a "work-around", it is required to materialize the results from the data you have. Though if your query will never get null from left join (be careful as data can change over the type in an app), or you are not accessing a property on an entity which can be nullable due to left join then you shouldn't need it. The same exception can also arise when database contains null but client side property cannot take null values (even without left-joins) due to mismatch in model. There is also a possibility that there is a bug in EF Core somewhere. So if you have a query which has result types correct based on expected data and seeing this exception please file a new issue.

It doesn't work if the target type is nullable but contains non nullable properties. Also there is no point in being forced to handle C# null safety inside a SQL query which doesn't have such concept. This turns what would have been a pretty standard SQL query with a couple of left joins into this mess:

            query.Select(user => new
            {
                User = user,
                Person = user.Person!,
                user.Substitute,
                user.Substitute!.OtherUser,
                user.Substitute.ExternalUser
            })
            .Select(record => new UserRecord
            {
                Id = record.User.Id,
                Firstname = record.Person!.Firstname,
                Lastname = record.Person.Lastname,
                Email = record.Person.Email,
                Salutation = record.Person.Salutation,
                Substitute = record.Substitute != null
                    ? new SubstituteRecord
                    {
                        Type = record.Substitute.Type,
#pragma warning disable S3358
                        OtherUser = record.OtherUser != null
                            ? new()
                            {
                                Id = record.OtherUser.Id,
                                Email = record.OtherUser.Person!.Email,
                                Salutation = record.OtherUser.Person.Email,
                                Firstname = record.OtherUser.Person.Firstname,
                                Lastname = record.OtherUser.Person.Lastname,
                            }
                            : null,
                        ExternalUser = record.ExternalUser != null
                            ? record.Substitute.ExternalUser
                            : null
                    }
                    : null
            });
GeoMarkou commented 1 year ago

We just upgraded to .Net 6 and this is an embarrassing complete disaster. There's no easy way to determine where this bug occurs other than scouring all 300+ LINQ expressions to see if we're using a leftjoin, and selecting on the result. This isn't even listed as a breaking change on the update page. The error message was completely useless and resulted in around 2 hours of debugging because I couldn't figure out what was going wrong (why would I expect a simple SELECT to be the cause of the error?). Furthermore, this behaviour is completely inconsistent even within the LINQ query itself, and doesn't follow expected C# paradigms. Consider the following query:

var userData = await (
    from u in db.user

    // left join
    join s in db.optional_user_settings_row
    on u.user_id equals s.user_id
    into leftjoin_u from u in leftjoin_u.DefaultIfEmpty()

    // s.suspended Doesn't error ???
    where u.user_id = UserIDToSearchFor && !s.suspended

    select new UserDataResult
    {
        name = u.display_name,
        // s.colourblind_enabled does error ???
        colourblind = s.colourblind_enabled,
    }
).ToArrayAsync();

This query errors because s is null, so it can't cast the resulting bool? into the expected not-null bool. However, why doesn't s.suspended in the where clause cause an error? Logically, the actual null value is s, but this seems to be behaving against expectations, as though we've just got a completely blank Object instead of a row, and everything inside of it is evaluating to null, but only sometimes. Not only that, but it's not even evaluating to default as it should be based on the DefaultIfEmpty() name. This makes no sense at all, and honestly we're seriously considering completely abandoning using Entity Framework LINQ because this isn't the first time an update has broken queries across our application with no warning or logical way to fix it. I ended up coming to the same workaround conclusions of casting to (bool?) or using == true but again this is crazy behaviour to any user and causes confusion.

I know Microsoft doesn't take suggestions, and this is a pipe dream, but the following syntax is what I would actually expect LINQ to work like.

var userData = await (
    from u in db.user

    // left join - new syntax
    // `s` is highlighted as POSSIBLY NULL
    // and will display a warning in Visual Studio if accessed directly below
    left join s in db.optional_user_settings_row
    on u.user_id equals s.user_id

    // Use null propagating operator since `s` is the actual null value
    // and we can EXPLICITLY HANDLE IT instead of automatically doing weird stuff
    where u.user_id = UserIDToSearchFor && (s?.suspended ?? false)

    select new UserDataResult
    {
        display_name = u.display_name,
        // Use coalescing to decide what the default should be
        colourblind_enabled = s?.colourblind_enabled ?? false,
    }
).ToArrayAsync();
blogcraft commented 1 year ago

Same problem in EF 7

darcast76 commented 11 months ago

I can't believe that after years, this error is still present. I have tons of LINQ queries that work correctly in .NET Framework 3.1, and I'm not sure how they might behave after an update. Has anyone found any workarounds? Is there at least one way to obtain a compile-time error instead of a runtime error?

dotVip commented 9 months ago

I have some issue when I move app from EF 2 to EF 7. I find solution that work for me

.SelectMany(d => d.Posts.DefaultIfEmpty(), (x, y) => new myModel { ...... somedata = y.somedata .GetType() == null ? "no data" : y.somedata, ...... });

junejasaket commented 5 months ago

This issue is a significant blocker for our transition from EF Core 3.1 to later versions. It fundamentally changes the expected behavior of left joins involving nullable types, which would necessitate extensive modifications in our codebase to introduce null checks to avoid these exceptions. This isn't just a minor inconvenience but a substantial overhaul of existing, stable code.

A configuration option to toggle this behavior would be highly beneficial, offering a compromise. This would allow teams who rely on the pre-5.0 behavior to upgrade without rewriting significant portions of their code, while others who prefer the new behavior could opt-in. Without such an option, we're effectively stuck, unable to leverage the advancements in newer EF Core versions.

It's crucial to address this issue to facilitate smoother transitions and maintain backward compatibility, especially for complex, enterprise-level applications.

Luigi6821 commented 5 months ago

Hi, I believe that this is duplicate of same issue I already reported [#30915] (still present on EF 8.0.1) This is definetively a blocker issue for moving from EF to EF Core. I have many LINQ query that raise same error.

Regards Luigi

ixetanet commented 2 months ago

Really hoping for a fix on this one...

Luigi6821 commented 2 months ago

Really hoping for a fix on this one...

Please vote it