Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: [Nav Prop Translation] EF7 generating incorrect sql with optional relationships #4205

Closed
mrahhal opened this issue Jan 3, 2016 · 9 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@mrahhal
Copy link

mrahhal commented Jan 3, 2016

Consider the following models:

    public class News
    {
        public int Id { get; set; }
        public string Title { get; set; }

        public Branch Branch { get; set; }
        public int? BranchId { get; set; }
    }

    public class NewsDto
    {
        public int Id { get; set; }
        public string Title { get; set; }
        public BranchDto Branch { get; set; }
    }

    public class Branch
    {
        public int Id { get; set; }
        public string Name { get; set; }

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

    public class BranchDto
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }

Here we have an optional relationship between News and Branch (1-to-many).

We're issuing the following query:

    context.News
        .Select(dto => new NewsDto()
        {
            Id = dto.Id,
            Title = dto.Title,
            Branch = dto.Branch == null ? null : new BranchDto()
            {
                Id = dto.Branch.Id,
                Name = dto.Branch.Name
            }
        }).ToList();

EF6

The generated query is:

SELECT 
[Extent1].[Id] AS [Id], 
[Extent1].[Title] AS [Title], 
CASE WHEN ([Extent2].[Id] IS NULL) THEN CAST(NULL AS int) ELSE 1 END AS [C1], 
[Extent1].[BranchId] AS [BranchId], 
[Extent2].[Name] AS [Name]
FROM  [dbo].[News] AS [Extent1]
LEFT OUTER JOIN [dbo].[Branches] AS [Extent2] ON [Extent1].[BranchId] = [Extent2].[Id]

This is how it should be: a left outer join after a CASE WHEN to handle null references to the Branches table.

EF7 (RC1)

The same query is generated as:

SELECT
[dto].[Id],
[dto].[Title],
[dto].[BranchId] IS NULL,
[dto].[BranchId],
[dto.Branch].[Name]
FROM [News] AS [dto]
INNER JOIN [Branch] AS [dto.Branch] ON [dto].[BranchId] = [dto.Branch].[Id]

I don't really understand why this is happening. The relationship is clearly optional so inner joins are just plain wrong. Other than that, the line [dto].[BranchId] IS NULL is giving a System.Data.SqlClient.SqlException: Incorrect syntax near the keyword 'IS' so I guess this isn't even valid sql.

This is only happening when the relationship is optional.

@mrahhal
Copy link
Author

mrahhal commented Jan 4, 2016

So I believe this is related to #3186

@smitpatel
Copy link
Member

[dto].[BranchId] IS NULL will be fixed by #3703

@mrahhal
Copy link
Author

mrahhal commented Jan 19, 2016

@rowanmiller #3703 is done and the LOJ issue is being tracked in #3186. So I believe we should close this?

@rowanmiller
Copy link
Contributor

@maumar anything you want to track on this issue... or is it good to be closed now (feel free to close if you are happy it's all covered).

@maumar
Copy link
Contributor

maumar commented Jan 20, 2016

I will keep it open for now to make sure that this specific case works (complex projection with navigations like that)

@rowanmiller
Copy link
Contributor

👍

@rowanmiller rowanmiller changed the title EF7 generating incorrect sql with optional relationships Query: [Nav Prop Translation] EF7 generating incorrect sql with optional relationships Jan 25, 2016
@mrahhal
Copy link
Author

mrahhal commented Feb 8, 2016

Just to add to this, I noticed similar problems occur even in the InMemory provider. Things like

context.News
    .Select(dto => new NewsDto()
    {
        Id = dto.Id,
        Title = dto.Title,
        Branch = dto.Branch == null ? null : new BranchDto()
        {
            Id = dto.Branch.Id,
            Name = dto.Branch.Name
        }
    }).ToList();

do not work as expected (return null).

So I guess a part of this is rooted in the common code and not something specific to the sql provider.

@maumar
Copy link
Contributor

maumar commented Feb 8, 2016

@mrahhal That is correct, navigation translation is done in the common part of the query pipeline - currently we always translate navigations into a LINQ join, which in memory have semantics of INNER JOIN and in relational get translated to INNER JOIN directly. The fix we are currently working on will translate optional navigations into SelectMany-GroupJoin-DefaultIfEmpty construct which behaves like LOJ

@maumar
Copy link
Contributor

maumar commented Feb 19, 2016

Fixed in c4e4447
Fix works fine for this particular scenario, however some more complex scenarios could still be broken or not efficient. E.g. Everything after optional navigation would be performed on the client.

@maumar maumar closed this as completed Feb 19, 2016
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
@ajcvickers ajcvickers modified the milestones: 1.0.0-rc2, 1.0.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

5 participants