[5.0.1] Query: Correctly compute ordering to be copied from outer in collection join

[smitpatel]

Resolves #23211
Resolves #23276

Adding quirk in a separate commit

Description

Same root cause results in two bugs:

1. When an entity type has two or more owned reference types which are mapped to different tables, then including the collection on such an entity type with a split query throws. Split queries are a new feature in 5.0.
2. When an entity type has an owned reference type mapped to a different table then including the collection in a split query with a single result generates wrong results which can lead to data corruption.

Customer impact

Either exception of data corruption when using new split query feature with owned reference types.

How found

We found this while adding MQ test coverage for owned types. It has not been customer reported yet, but we believe that customers will hit this when they start using split queries (which is expected to get very high adoption) and may suffer data corruption.

Test coverage

This PR includes test for the affected scenario. A more thorough testing is being added in #23212

Regression?

No, split query was added in 5.0 only

Risk

Low. The fix only expands scope of pattern match so previously working queries would continue working.

[ajcvickers]

@smitpatel Tactics feedback on, "While adding additional test coverages," is that we should bring this back if we have customer reports. If we are very confident that this is something we should fix even without customer reports, then we should make the case for that. Either way we can bring back to tactics when we have more info.

[smitpatel]

According to #23276 this is causing incorrect results to customer which will corrupt database without noticing. Let's discuss in triage. I am also fine waiting for a customer to hit the issue.

[ajcvickers]

Approved by Tactics for 5.0.1. However, we should loop back with Tactics once we have customer reports on this to confirm this was the correct choice.

[ghost]

Hello @smitpatel!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[smitpatel]

@msftbot merge this after 1 hour

[ghost]

Hello @smitpatel!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I won't merge this pull request until after the UTC date Tue, 17 Nov 2020 20:43:11 GMT, which is in 1 hour

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[ghost]

Apologies, while this PR appears ready to be merged, it looks like release/5.0 is a protected branch and I have not been granted permission to perform the merge.

[ghost]

Apologies, while this PR appears ready to be merged, it looks like release/5.0 is a protected branch and I have not been granted permission to perform the merge.

[ghost]

Apologies, while this PR appears ready to be merged, it looks like release/5.0 is a protected branch and I have not been granted permission to perform the merge.

[smarts]

Are there any limitations to this fix, like a certain depth in the object graph?
I have an EF configuration that is still producing the error from #23311.
I'm on an EF Core version that should have this fix, 5.0.6

[smitpatel]

@smarts - There are no limitations to this fix. Though the error msg which arises from this bug is generic and can happen in other totally different scenarios too. Please file a new issue with repro code so that we can investigate.