-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[rc2] Fix DefaultIfEmpty and nullability within SelectMany selector #36238
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
Conversation
419975b to
54a1bc9
Compare
Pull request was converted to draft
54a1bc9 to
a749bf5
Compare
artl93
left a comment
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.
RC2
| Assert.Equal( | ||
| CoreStrings.ExpressionParameterizationExceptionSensitive( | ||
| "value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass175_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"), | ||
| "value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass177_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"), |
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.
Consider using Contains to avoid having a compiler-generated string in the assert
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.
What makes this slightly tricky is that the compiler-generated component is integrated inside the CoreStrings template etc... If it starts happening frequently we can think about a better way...
Fixes #35950
Description
See #35950 (comment) for a detailed explanation of what's going on. tl;dr a COALESCE optimization done in 9.0 exposed a previously-existing, latent bug with columns not being considered nullable though they're after DefaultIfEmpty.
Customer impact
Some queries involving the DefaultIfEmpty() operator which worked in EF 8.0 started failing in EF 9.0.
Note that various other bugfixes to DefaultIfEmpty have been done in 10.0 (#36208, #33343, #19095) - this continues those fixes to substantially improve DefaultIfEmpty support in general.
How found
User-reported
Regression
Yes, from 8.0 to 9.0.
Testing
Added
Risk
Low, targeted fix for the specific scenario.