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

Unstable sort for Correlated_collection_with_top_level_FirstOrDefault test #15164

Closed
cincuranet opened this issue Mar 26, 2019 · 7 comments · Fixed by #18193
Closed

Unstable sort for Correlated_collection_with_top_level_FirstOrDefault test #15164

cincuranet opened this issue Mar 26, 2019 · 7 comments · Fixed by #18193
Assignees
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported good first issue This issue should be relatively straightforward to fix. type-bug
Milestone

Comments

@cincuranet
Copy link
Contributor

The test Correlated_collection_with_top_level_FirstOrDefault does not have an explicit unique sort and hence the loop will trigger false negative when the order is different.

To be precise, the query returns 2 rows with IDs 7 and 8. And the order isn't defined because the Nickname, SquadId and FullName are same (i.e. here).

@ajcvickers
Copy link
Member

@cincuranet Can you submit a PR to fix this?

@cincuranet
Copy link
Contributor Author

cincuranet commented Apr 1, 2019 via email

@divega divega added this to the Backlog milestone Apr 1, 2019
@divega divega added help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. area-test labels Apr 1, 2019
@j-rewerts
Copy link
Contributor

I'm open to handling this. I just want to make sure @cincuranet hasn't started on it yet.

@cincuranet
Copy link
Contributor Author

cincuranet commented Apr 20, 2019 via email

@j-rewerts
Copy link
Contributor

@cincuranet Which version of EF Core are you using?

@j-rewerts
Copy link
Contributor

It looks like this test is related to #15043 . Digging deeper, it looks like this test was disabled for the query rewrite effort. @maumar Would it be best for me to hold off on this until work is further along?

@maumar
Copy link
Contributor

maumar commented Apr 21, 2019

@j-rewerts we can take the PR now. The test has been disabled preemptively, as the first phase of query rewrite changes will break it, but for now the scenario works. You can enable the test on your box to make sure the fix is correct and disable it back for the PR. Once #15043 is fixed we can get the test online and it will have the proper fix already.

@divega divega added good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. good first issue This issue should be relatively straightforward to fix. labels May 31, 2019
@ajcvickers ajcvickers added good first issue This issue should be relatively straightforward to fix. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. labels Aug 5, 2019
@ajcvickers ajcvickers modified the milestones: Backlog, 3.1.0 Sep 4, 2019
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 2, 2019
maumar added a commit that referenced this issue Oct 2, 2019
resolves #14550
resolves #15164
resolves #15994

added regression tests for #14671
added regression test for #17852
converted remaining tests in Gears of War into AssertQuery pattern (part of #12501)
maumar added a commit that referenced this issue Oct 2, 2019
resolves #14550
resolves #15164
resolves #15994

added regression tests for #14671
added regression test for #17852
converted remaining tests in Gears of War into AssertQuery pattern (part of #12501)
@maumar maumar closed this as completed in 1bd4e85 Oct 3, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview2 Oct 24, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0-preview2, 3.1.0 Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported good first issue This issue should be relatively straightforward to fix. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants