Efcore: Query: INNER JOIN generated for navigation traversals from principal to dependents

[Bartmax]

This is very counter intuitive, I'm not sure what's going on.
My goal was to get the 4 database entries, with null on Logo when Details is null.
I tried different approaches and i found it very inconsistent.

So obvious question is: is it possible to get all rows, left join with default if empty?
Question 2: Am I doing something wrong or this is a bug?

Database has 4 entries, 2 with null details.

this query returns 2 results:

var query1 = await context.Games
    .Include(g=>g.Details)
    .ThenInclude(d=>d.Logo)
    .Select(g=> new { Logo = g.Details.Logo, g.Title, g.GameId })
    .ToListAsync();

---

this query returns 4 results:

var query2 = await context.Games
    .Include(g=>g.Details)
    .ThenInclude(d=>d.Logo)
    .Select(g=> new { g.Title, g.GameId })
    .ToListAsync();

---

this query throws:
InvalidOperationException: Operation is not valid due to the current state of the object. GetArgument

var query3 = await context.Games
    .Include(g=>g.Details)
    .ThenInclude(d=>d.Logo)
    .DefaultIfEmpty()
    .Select(g=> new {  Logo = g.Details.Logo, g.Title, g.GameId })
    .ToListAsync();

---

this query throws:
SqlException: The column prefix 't10' does not match with a table name or alias name used in the query. No column name was specified for column 1 of 't2'. Invalid column name 'Title'. Invalid column name 'GameId'.

 var query4 = await context.Games
    .Include(g=>g.Details)
    .ThenInclude(d=>d.Logo)
    .DefaultIfEmpty()
    .Select(g=> new { Logo = (GameFile)null, g.Title, g.GameId })
    .ToListAsync();

---

this query returns 4 elements, same as query2. (logo is explicit set as null):

var query5 = await context.Games
    .Include(g=>g.Details)
    .ThenInclude(d=>d.Logo)
    .Select(g=> new { Logo = (GameFile)null, g.Title, g.GameId })
    .ToListAsync();

---

note: using g.Details?.Logo on lambda does not compile.

[Bartmax]

I'm think the problem lies when specifying the navigation inside the Select the query switches from LEFT JOIN to JOIN hence my attempt to use DefaultIfEmpty.

[rowanmiller]

@Bartmax can you share your entity classes and context code

[Bartmax]

I'll try:

public class Game : ISortable, IAuditable
{
        public int GameId { get; set; }
        public GameDetails Details { get; set; }
        ...
}

 public class GameDetails
{
        public int GameDetailsId { get; set; }
        public int GameId { get; set; }
        public GameFile Logo { get; set; }
        ...
}

public class GameFile : IFileData
{
        private const string folder = "Games";

        GameFile() { /* EF */ }

        public GameFile(FileData fileData)
        {
            GameFileId = fileData.FileDataId;
            Filename = fileData.Filename;
            OriginalFilename = fileData.OriginalFilename;
            ContentType = fileData.ContentType;
        }

        [Key]
        public Guid GameFileId { get; private set; }

        [Required]
        public string ContentType { get; set; }

        [Required]
        public string Filename { get; set; }

        public static string Folder => folder;
        string IFileData.Folder => folder;

        Guid IFileData.FileDataId => GameFileId;

        public string OriginalFilename { get; set; }

    }

builder.Entity<Game>(e =>
{
                e.Property(p => p.Title).IsRequired();
                e.Property(p => p.Slug).IsRequired();
                e.HasAlternateKey(p => p.Slug);
                e.HasIndex(p => p.Slug);
                e.Property(p => p.GameColor).IsRequired().HasDefaultValue("000");
});

builder.Entity<GameFile>().Property(p => p.Filename).IsRequired(true);

Let me know if you need something else, I removed a lot of stuff to make it readable, hope I included all details that matters.

[gdoron]

I'm not sure how Include works with Select.
My understanding from other issues that Select overrides all the Includes.
Did I misunderstood?

[Bartmax]

@gdoron I don't think select overrides includes. not at least in my experience using it.

[rowanmiller]

The Includes should be a no-op in the queries shown. The includes mean "when you materialize a Game, also bring back the Detail and Logo"... but then the query is changed to not return Games, so they have no effect. So the Includes are not needed, but the query should still work.

[Bartmax]

@rowanmiller great information to know!! thanks. not sure how I missed that but whatever.
thanks @gdoron :+1:, this is basic that's very handy to be aware of.

[maumar]

@Bartmax problems seems to be in a way that the model is setup.

 public class GameDetails
{
        public int GameDetailsId { get; set; }
        public int GameId { get; set; }
        public GameFile Logo { get; set; }
        ...
}

since GameId is a non-nullable int, EF assumes that the navigation between Game and GameDetails is a required one. Therefore INNER JOIN is produced for that navigation, which results in 2 results being "eaten". Here is the SQL generated for the first query:

SELECT [g.Details].[GameDetailsId], [g.Details].[GameId], [g.Details].[LogoGameFileId], [g.Details.Logo].[GameFileId], [g.Details.Logo].[ContentType], [g.Details.Logo].[Filename], [g.Details.Logo].[OriginalFilename], [g].[Title], [g].[GameId]
FROM [Games] AS [g]
INNER JOIN [GameDetails] AS [g.Details] ON [g].[GameId] = [g.Details].[GameId]
LEFT JOIN [GameFiles] AS [g.Details.Logo] ON [g.Details].[LogoGameFileId] = [g.Details.Logo].[GameFileId]
ORDER BY [g.Details].[LogoGameFileId]

This can be fixed by setting GameId to be int? - you will see correct 4 results returned. Note however that due to https://github.com/aspnet/EntityFramework/issues/4588 everything coming after first optional navigation is done on the client so in fact we will send 2 queries to the database:

SELECT [g].[GameId], [g].[Title], [g.Details].[GameDetailsId], [g.Details].[GameId], [g.Details].[LogoGameFileId]
FROM [Games] AS [g]
LEFT JOIN [GameDetails] AS [g.Details] ON [g].[GameId] = [g.Details].[GameId]
ORDER BY [g].[GameId]

and

SELECT [g.Details.Logo].[GameFileId], [g.Details.Logo].[ContentType], [g.Details.Logo].[Filename], [g.Details.Logo].[OriginalFilename]
FROM [GameFiles] AS [g.Details.Logo]

and patch the results on the client.

Problems with queries 3 and 4 are legitimate bugs and I filed separate issue for them: https://github.com/aspnet/EntityFramework/issues/6207

[maumar]

After giving it some thought, I think that the original problem is a legitimate bug also - we should not be assuming that navigation is optional when we query from principal to dependent. It is perfectly fine to have a game (principal) without the game detail (dependent), even when the FK on the game detail is not nullable

[maumar]

Moving the milestone to 1.0.1 as we currently return incorrect results

[Bartmax]

Thanks for the detailed explanation. I'm glad it also helped to discover some bugs.

[maumar]

@divega ping regarding triage

[gdoron]

@rowanmiller

The Includes should be a no-op in the queries shown. The includes mean "when you materialize a Game, also bring back the Detail and Logo"... but then the query is changed to not return Games, so they have no effect. So the Includes are not needed, but the query should still work.

I think throwing is better than no-op, it's clear that the author meant something, that EF can not do, ignoring his mistake by swallowing the error can lead to unexpected results.

[Bartmax]

@gdoron oh no please don't throw. a runtime warning or just no op.

[maumar]

We can't throw at this point - it would be a breaking change for people who produced those queries based on 1.0.0

[divega]

@maumar can you help us confirm the scope of the bug (and of the potential fix), e.g. does it only affect 1:0..1 relationships? Does it affect Include() as well?

[darkurse]

+1 for a runtime warning as it is a confusing behaviour.
Using the ILogger from DbContext perhaps ?

[maumar]

@divega it only affects 1:0..1 relationships - collections are fine, includes are fine.

[divega]

@darkurse @Bartmax @gdoron note that we already have CoreEventId.IncludeIgnoredWarning although I am not 100% sure from the top of my mind if this case is caught. If you happen to test it and a warning isn't issued for a case in which you think it should, please file a separate bug.

[divega]

@maumar could we fix the issues with null compensation when functions are evaluated in memory for 1.0.1? If we can't then we are not completely comfortable with taking this in 1.0.1.

[divega]

Pulling from 1.0.1 into 1.1.0 because the fix has grown big enough that we are not longer confident we should include it in a patch.

cc @maumar @Eilon

[maumar]

Fixed in 2888a861bb05aaac870b73d95433235600253bf2

[rousselleg]

This is technically a different issue, but it is related to this thread. It seems that you are not able pull certain properties out of the dependent objects in the query. Instead you need to return the entire object from the database and then inspect the properties you are concerned about.

This code fails :
await _repo .TableNoTracking .Where(x => x.Username == profileName) .Select(x => new { IsAdmin = x.User != null ? x.User.IsAdmin : false }) .FirstOrDefaultAsync()

this code works fine, but we are forced to return the entire object.
await _repo .TableNoTracking .Where(x => x.Username == profileName) .Select(x => new { User = x.User }) .FirstOrDefaultAsync()