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

[release/6.0] Query: Convert single result subquery comparison to null to Any operation #27284

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

smitpatel
Copy link
Contributor

@smitpatel smitpatel commented Jan 26, 2022

Resolves #26744
A better fix for #18476

Initial fix for #18476 (in 6.0) 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.

Description

A faulty optimization in EF Core converts a subquery comparison to null to any operation making it sequence check. In case when the sequence can return null value as element, the optimization is incorrect.

Customer impact

Users running a query where this optimization would incorrectly kick in, will get incorrect results.

How found

Customer reported on 6.0

Regression

Yes. From 5.0 The regression was caused by faulty optimization added in 6.0 as enhancement.

Testing

There are existing test verifying the optimization (in correct scenario), Added additional tests for user scenario where optimization was incorrect. Some tests have different SQL now as optimization was incorrect but due to the data, it was still giving correct results.

Risk

Low risk. Optimization is applied accurately in the cases where it is correct. This has potential to break some queries though they are supposed to be giving incorrect results. Also added quirk to revert to earlier behavior.

…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 marked this pull request as ready for review January 26, 2022 04:29
@smitpatel smitpatel requested a review from a team January 26, 2022 04:29
@smitpatel smitpatel added this to the 6.0.x milestone Jan 26, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.3 Jan 27, 2022
@wtgodbe wtgodbe merged commit b7e5c13 into release/6.0 Feb 2, 2022
@wtgodbe wtgodbe deleted the smit/6.0/issue26744_take2 branch February 2, 2022 17:37
@ajcvickers ajcvickers removed this from the 6.0.3 milestone Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants