[6.0.2] Query: Update column expression correctly when lifting joins from group by aggregate subquery #27109
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smitpatel
force-pushed
the
smit/snowisbad
branch
from
c20b140
to
5a89f0a
Compare
maumar
reviewed
Jan 7, 2022
| var columnExpressionReplacingExpressionVisitor = | ||
| new ColumnExpressionReplacingExpressionVisitor(subquery, _selectExpression); | ||
| AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27083", out var enabled2) && enabled2 |
There was a problem hiding this comment.
nit: maybe we should switch to enabled27083 convention for these? (same with useOldBehavior in other places)
maumar
approved these changes
Jan 7, 2022
smitpatel
force-pushed
the
smit/newyearnewme
branch
from
4546961
to
490b5ea
Compare
…up by aggregate subquery When replacing columns, we used the outer select expression which had additional joins from previous term whose aliases match tables in current join and it got updated with wrong table. The fix is to utilize the original tables when replacing columns. These column replacement is to map the columns from initial tables of group by from subquery to outer group by query. Resolves #27083
smitpatel
force-pushed
the
smit/snowisbad
branch
from
5a89f0a
to
95b291d
Compare
ajcvickers
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When replacing columns, we used the outer select expression which had additional joins from previous term whose aliases match tables in current join and it got updated with wrong table.
The fix is to utilize the original tables when replacing columns. These column replacement is to map the columns from initial tables of group by from subquery to outer group by query.
Resolves #27083
This PR is targeting another PR in release/6.0. If the earlier PR is not approved, I will rebase this PR direct on 6.0. The dependency between PR related to group by is to avoid unintentional side-effects between different patch fixes.
Description
When expanding navigation in aggregate operation in projection after GroupBy operator, we lift the joins generated from navigation to main group by query. In certain conditions we end up generating SQL referencing wrong tables which could generate incorrect results.
Customer impact
Customer will get incorrect results without any exception.
How found
Customer reported on 6.0.1
Regression
No. The scenario of navigations inside aggregate operation was not supported prior to 6.0
Testing
Added test for user mentioned scenario which covers the root cause of the issue. (matching alias)
Risk
Low risk. Change only affects above scenario. Also added quirk to revert back to earlier behavior in case working scenario breaks.