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

FirstOrDefault after custom projection == null cannot be translated #18476

Closed
groege opened this issue Oct 21, 2019 · 10 comments · Fixed by #25914
Closed

FirstOrDefault after custom projection == null cannot be translated #18476

groege opened this issue Oct 21, 2019 · 10 comments · Fixed by #25914
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-enhancement
Milestone

Comments

@groege
Copy link

groege commented Oct 21, 2019

Steps to reproduce

var queryA = db.MyEntity.Select(x => new MyEntityModel {
    ModelPropA = x.Children.Where(y => y.Type == Types.A).FirstOrDefault(),
    ModelPropB = x.Children.Where(y => y.Type == Types.B).FirstOrDefault(),
});
var queryB = queryA.Select(x => new MyEntityModel {
    ModelPropA = x.ModelPropA,
});

queryA.ToList() //This is working
queryB.ToList() //Throws System.InvalidOperationException: 'Sequence contains more than one element'

//EDIT:
queryA.Where(x => x.ModelPropA == null).ToList() //Throws the System.InvalidOperationException as well

StackTrace

Further technical details

EF Core version:
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL 3.0.0
Target framework: NET Core 3.0
Operating system: WIN10
IDE: Visual Studio 2019 16.4.0 Preview 1.0

@ajcvickers ajcvickers added this to the Backlog milestone Oct 21, 2019
@ajcvickers
Copy link
Contributor

@groege Please post a small, runnable project/solution or complete code listing that demonstrates the issue. This will ensure that we can reproduce what you are seeing.

@groege
Copy link
Author

groege commented Oct 22, 2019

ExceptionRepro.zip

So I apologize - queryB actually works - I did have an error in my code (Which I found by doing the repro).

But queryA.Where(x => x.ModelPropA == null).ToList() is not working, it sais it cannot be translated.
See queryC in the repro project.

Thanks for your time!!

@smitpatel
Copy link
Contributor

Simpler query

var query = db.Blogs.Where(e => db.Blogs.Select(b => new { b.Id }).FirstOrDefault() == null).ToList();

@smitpatel smitpatel changed the title System.InvalidOperationException: 'Sequence contains more than one element': When sub query contains FirstOrDefault FirstOrDefault after custom projection == null cannot be translated Oct 22, 2019
@groege
Copy link
Author

groege commented Oct 23, 2019

@smitpatel thanks!
Though that query doesn't seem simpler to me ^^ - looks convoluted to me (compared to just using a normal where clause)

@smitpatel
Copy link
Contributor

@groege - Your normal where clause has custom projection preceding it. The simpler query is for repro and not a work-around.

@smitpatel
Copy link
Contributor

This is anonymous type comparison.
Duplicate of #14672

@smitpatel smitpatel removed this from the 6.0.0 milestone Nov 6, 2020
@smitpatel smitpatel reopened this Nov 6, 2020
@ajcvickers ajcvickers added this to the 6.0.0 milestone Nov 7, 2020
@smitpatel
Copy link
Contributor

smitpatel commented Aug 21, 2021

Notes:

  • If the result type of FirstOrDefault is non-nullable type then == null is always false (& inverse for != null) Compiler generates error
  • For nullable result type result of OrDefault family will be null only if the sequence is null. So we can rewrite this check to Any on sequence itself. For != null check, SingleOrDefault could twist things but following our philosophy or server eval, we treat it same as FirstOrDefault in subquery.
  • This could also help us in translating entity equality with subquery & composite key (if we are not doing already)

smitpatel added a commit that referenced this issue Sep 7, 2021
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement and removed type-bug labels Sep 7, 2021
@smitpatel
Copy link
Contributor

smitpatel commented Nov 18, 2021

For nullable result type result of OrDefault family will be null only if the sequence is null. So we can rewrite this check to Any on sequence itself. For != null check, SingleOrDefault could twist things but following our philosophy or server eval, we treat it same as FirstOrDefault in subquery.

This is only true if the projection type is non-nullable hence null will produced from empty sequence only. If the result type is nullable then a non-empty sequence can also give null.

@smitpatel smitpatel reopened this Nov 18, 2021
@smitpatel
Copy link
Contributor

Re-activating as we didn't really fix this issue.

@smitpatel smitpatel removed this from the 6.0.0 milestone Nov 18, 2021
@smitpatel smitpatel removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Nov 18, 2021
@ajcvickers ajcvickers added this to the 7.0.0 milestone Nov 19, 2021
smitpatel added a commit that referenced this issue Jan 26, 2022
…tion

Resolves #26744
A better fix for #18476

Initial fix for #18476 assumed that whenever we have single result operation compared to null, it will only be true if the result of single result is default when sequence is empty. This was correct for the query in the issue tracker which had anonymous type projection.
Anonymous type is never null as long as there is data, it can be only null value when default is invoked i.e. empty sequence.
Hence we added optimization for that but it didn't restrict to just anonymous type.
For entity type projection when entity is not nullable, the same logic holds true. This helped us translate queries which wouldn't work with entity equality due to composite key from a subquery.
But optimization was incorrect for the result which can be null (nullable scalar or nullable entity) as an non-empty sequence can have first result to be null which can match.

The improved fix avoids doing the unrestricted optimization during preprocessing phase. Instead we moved the logic to translation phase where we can evaluate the shape of the projection coming out subquery. Now we only apply optimization for non-nullable entity and anonymous type. Scalar comparison will work by comparing to null and nullable entity will work if entity equality covers it. It will start throwing error if composite key though earlier version possibly generated wrong results for it.
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 26, 2022
@smitpatel smitpatel modified the milestones: 7.0.0, 6.0.x Jan 26, 2022
@smitpatel
Copy link
Contributor

@ajcvickers - While this is not really patch issue, it is getting fixed in 6.0 branch as fixing bug for #26744 Not sure how to set milestone on this one now.

@ajcvickers ajcvickers modified the milestones: 6.0.x, 6.0.3 Jan 26, 2022
wtgodbe pushed a commit that referenced this issue Feb 2, 2022
…tion (#27284)

Resolves #26744
A better fix for #18476

Initial fix for #18476 assumed that whenever we have single result operation compared to null, it will only be true if the result of single result is default when sequence is empty. This was correct for the query in the issue tracker which had anonymous type projection.
Anonymous type is never null as long as there is data, it can be only null value when default is invoked i.e. empty sequence.
Hence we added optimization for that but it didn't restrict to just anonymous type.
For entity type projection when entity is not nullable, the same logic holds true. This helped us translate queries which wouldn't work with entity equality due to composite key from a subquery.
But optimization was incorrect for the result which can be null (nullable scalar or nullable entity) as an non-empty sequence can have first result to be null which can match.

The improved fix avoids doing the unrestricted optimization during preprocessing phase. Instead we moved the logic to translation phase where we can evaluate the shape of the projection coming out subquery. Now we only apply optimization for non-nullable entity and anonymous type. Scalar comparison will work by comparing to null and nullable entity will work if entity equality covers it. It will start throwing error if composite key though earlier version possibly generated wrong results for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants