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

OData with custom modelbuilder attached to Entity framework produces a duplicate left join #2205

Open
PhbsSmn opened this issue Jun 18, 2020 · 10 comments
Assignees

Comments

@PhbsSmn
Copy link

PhbsSmn commented Jun 18, 2020

I'm having an issue that I first didn't notice because my development machine was fast and my test set small. When publishing to a slower machine with a large dataset, I noticed that when an expand is given with a custom modelbuilder it creates a duplicat join and in turn creating a cross join.

Assemblies affected

Microsoft.AspNetCore.OData 7.4.1

Reproduce steps

I created a small demo project because the issue is easy to reproduce once you know where to look for. Source available on this location https://github.com/PhbsSmn/ExpandIssue.

  1. Create a local sql database, I've added 2 sql scripts CreateDatabase.sql and AddData.sql
  2. Start a profiler so that you can see that generated sql scripts
  3. Trigger an odata call: https://localhost:5001/odata/MasterTables?$filter=Id eq 4&$expand=Childs
  4. Get the output from the profiler

Expected result

DECLARE @__TypedProperty_2 nvarchar(4000) = N'22bfc7f7-1f8a-4a92-b56b-c25cbb0742bd'
DECLARE @__TypedProperty_3 int = 101
DECLARE @__TypedProperty_0 int = 4

SELECT
    [t].[Id], [t].[Name], 
    [c].[Id], [c].[Created], [c].[Value], @__TypedProperty_2,
    CAST(1 AS bit)
FROM
    (SELECT TOP(@__TypedProperty_3) [m].[Id], [m].[Name]
     FROM [MasterTable] AS [m]
     WHERE [m].[Id] = @__TypedProperty_0
     ORDER BY [m].[Id]) AS [t]
    LEFT JOIN [ChildTable] AS [c] ON [t].[Id] = [c].[MasterTableId]
ORDER BY
    [t].[Id], [c].[Id]

Actual result

DECLARE @__TypedProperty_2 nvarchar(4000) = N'22bfc7f7-1f8a-4a92-b56b-c25cbb0742bd'
DECLARE @__TypedProperty_3 int = 101
DECLARE @__TypedProperty_0 int = 4

SELECT
    [t].[Id], [t].[Name], 
    [c].[Id], [c].[Created], [c].[Value], @__TypedProperty_2,
    [c0].[Id], [c0].[Created], [c0].[Value], CAST(1 AS bit)
FROM
    (SELECT TOP(@__TypedProperty_3) [m].[Id], [m].[Name]
     FROM [MasterTable] AS [m]
     WHERE [m].[Id] = @__TypedProperty_0
     ORDER BY [m].[Id]) AS [t]
    LEFT JOIN [ChildTable] AS [c] ON [t].[Id] = [c].[MasterTableId]
    LEFT JOIN [ChildTable] AS [c0] ON [t].[Id] = [c0].[MasterTableId]
ORDER BY
    [t].[Id], [c].[Id], [c0].[Id]
@Sreejithpin Sreejithpin self-assigned this Jun 23, 2020
@Sreejithpin Sreejithpin transferred this issue from OData/odata.net Jun 23, 2020
@Sreejithpin
Copy link
Contributor

Hi,
We are investigating on this issue. Meanwhile for .net 5 Entity framework has significant updates and could try that

@PhbsSmn
Copy link
Author

PhbsSmn commented Jun 24, 2020

Hi,

I created a .net5 branch with the new entity framework updates in prerelease but the same issue occurs. I'll keep this sample project for trying out the suggested updates.

@ajcvickers
Copy link

@smitpatel Does this look like an EF issue with the translation?

@btull89
Copy link

btull89 commented Mar 29, 2021

@ajcvickers, @Sreejithpin: Any updates on this? I'm having the same issue with .net core 3.1. According to @PhbsSmn upgrading to .net 5 will not resolve the issue here.

@PhbsSmn are you still having this problem?

@smitpatel
Copy link

This looks like issue with modifications done by user, perhaps OData team can verify.
Specifically following code
https://github.com/PhbsSmn/ExpandIssue/blob/665d6655cc559649dc641d877d5399c3b7b50124/ExpandIssue/OData/MasterTablesController.cs#L23-L40

It construct custom object with nested subquery. And using expand in OData query would probably add another subquery for Children, essentially duplicating the join in the LINQ hence generated SQL also has join twice.

@btull89
Copy link

btull89 commented Mar 30, 2021

I appreciate the reply @smitpatel . I'm thinking that my issue may be with Automapper.
I removed Automapper and view models from the equation and did not experience the duplicate left join.

I think my issue is also similar to #863.
From Automapper documentation Without explicit instructions, AutoMapper will expand all members in the result. I believe this is somehow causing a duplicate left join that the underlying linq provider isn't detecting causing another left join to occur. It is definitely causing the left join whenever I'm not even expanding the child collection.

I was able to get around the issue by following the advice found in that issue using Explicit Expansion.
cfg.CreateMap<SalesOrderHeader, SalesOrderDto>().ForMember(dest => dest.SalesOrderLines, opt => opt.ExplicitExpansion());

@smitpatel
Copy link

It is definitely causing the left join whenever I'm not even expanding the child collection.

That certainly points in the direction of automapper or custom query generated causing additional join.

From EF Core side, this is a collection navigation so it is a subquery. Every occurrence of it will cause a join since they are distinct collections. As far my understanding of OData goes, if you ask for expand it will add a collection subquery explicitly causing at least 1 join. So if any query given to OData which already has a subquery for children, and using expand afterwards, 2 joins expected and nothing EF Core or OData can change about it particularly.

@btull89
Copy link

btull89 commented Mar 30, 2021

That explanation sounds spot on @smitpatel. Thank you for your time!

@PhbsSmn
Copy link
Author

PhbsSmn commented Apr 9, 2021

That explanation sounds spot on @smitpatel. Thank you for your time!

So this means if you introduce automapper it gets solved?

@btull89
Copy link

btull89 commented Apr 9, 2021

@PhbsSmn I think AutoMapper was causing my issue in a way. I was already using AutoMapper, and I started using AutoMapper.Extensions.OData to resolve my issue.

Now I'm not sure if it was truly AutoMapper's fault or an issue with OData or EF Core.
Our 10+ projects did not see this issue on .net core 2.1 even though we were using AutoMapper, and we only had this issue when we upgraded to .net core 3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants