-
Notifications
You must be signed in to change notification settings - Fork 159
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
fix duplicate join issues #993
fix duplicate join issues #993
Conversation
7902c1e
to
03294f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some unit tests here
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
f808208
to
ff8fa00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great to add a test that looks like the customer scenario described in the issue, i.e.: applying OData query options to an IQueryable that is the result of a .Select() mapping from one type to another, where the input comes from a DbSet with an Includes call on a navigation property.
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs
Show resolved
Hide resolved
d3d29e5
to
9e13363
Compare
This issue leads to duplicate joins being generated, like the one below, when we execute such a query
RawIncidents?$expand=alerts
SelectExpandWrapper
uses theInstance
property which contains all the properties of a type including the structural and navigation properties. We then again add the navigation properties to theContainer
property. So it's like the navigation properties get added twice and EF doesn't do deduplication of custom expressions (by design) which leads to duplicate joins being generated.The solution here was to only write the structural properties to the
Instance
and leave out the navigation properties which get added to theContainer
property.To the
ProjectElement
method in theSelectExpandBinder
class, I have made some updates to ensure that theInstance
that gets created is in this form:That's, it only has structural properties.
Another change was to check that the QueryProvider is an EF query provider.. These changes are only done when we are dealing with an EF QueryProvider. If not, then the expression is built the way it has always been built.