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

[automated] Merge branch 'release/8.0' => 'main' #33302

Merged
merged 29 commits into from
Mar 13, 2024

Conversation

dotnet-maestro-bot
Copy link
Contributor

I detected changes in the release/8.0 branch which have not been merged yet to main. I'm a robot and am configured to help you automatically keep main up to date, so I've opened this PR.

This PR merges commits made on release/8.0 by the following committers:

  • maumar
  • vseanreesermsft
  • AndriySvyryd
  • dotnet-bot

Instructions for merging from UI

This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, not a squash or rebase commit.

merge button instructions

If this repo does not allow creating merge commits from the GitHub UI, use command line instructions.

Instructions for merging via command line

Run these commands to merge this pull request from the command line.

git fetch
git checkout release/8.0
git pull --ff-only
git checkout main
git pull --ff-only
git merge --no-ff release/8.0

# If there are merge conflicts, resolve them and then run git merge --continue to complete the merge
# Pushing the changes to the PR branch will re-trigger PR validation.
git push https://github.com/dotnet-maestro-bot/EntityFrameworkCore HEAD:merge/release/8.0-to-main
or if you are using SSH
git push [email protected]:dotnet-maestro-bot/EntityFrameworkCore HEAD:merge/release/8.0-to-main

After PR checks are complete push the branch

git push

Instructions for resolving conflicts

⚠️ If there are merge conflicts, you will need to resolve them manually before merging. You can do this using GitHub or using the command line.

Instructions for updating this pull request

Contributors to this repo have permission update this pull request by pushing to the branch 'merge/release/8.0-to-main'. This can be done to resolve conflicts or make other changes to this pull request before it is merged.

git checkout -b merge/release/8.0-to-main main
git pull https://github.com/dotnet-maestro-bot/EntityFrameworkCore merge/release/8.0-to-main
(make changes)
git commit -m "Updated PR with my changes"
git push https://github.com/dotnet-maestro-bot/EntityFrameworkCore HEAD:merge/release/8.0-to-main
or if you are using SSH
git checkout -b merge/release/8.0-to-main main
git pull [email protected]:dotnet-maestro-bot/EntityFrameworkCore merge/release/8.0-to-main
(make changes)
git commit -m "Updated PR with my changes"
git push [email protected]:dotnet-maestro-bot/EntityFrameworkCore HEAD:merge/release/8.0-to-main

Contact .NET Core Engineering if you have questions or issues.
Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/master/scripts/GitHubMergeBranches.ps1.

dotnet-bot and others added 28 commits February 6, 2024 23:03
Merge from public release/8.0 to internal/release/8.0 and resolve conflicts if necessary
…es (dotnet#33212)

Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes dotnet#32911
….0-2024-03-12-1058

Merging internal commits for release/7.0
….0-2024-03-12-1101

Merging internal commits for release/8.0
Copy link
Contributor

@dotnet-policy-service dotnet-policy-service bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approving branch merge.

@ajcvickers ajcvickers merged commit 4a3523a into dotnet:main Mar 13, 2024
1 of 6 checks passed
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.

6 participants