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

Average_Grouped_from_LINQ_101 is non-deterministic #27155

Closed
roji opened this issue Jan 10, 2022 · 1 comment · Fixed by #27280 or #28650
Closed

Average_Grouped_from_LINQ_101 is non-deterministic #27155

roji opened this issue Jan 10, 2022 · 1 comment · Fixed by #27280 or #28650
Assignees
Labels
area-query area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Jan 10, 2022

In:

public virtual Task Average_Grouped_from_LINQ_101(bool async)
    => AssertQuery(
        async,
        ss => from p in ss.Set<ProductForLinq>()
              group p by p.Category
              into g
              select new { Category = g.Key, AveragePrice = g.Average(p => p.UnitPrice) },
        ss => from p in ss.Set<ProductForLinq>()
              group p by p.Category
              into g
              select new { Category = g.Key, AveragePrice = Math.Round(g.Average(p => p.UnitPrice) - 0.0000005m, 6) });

There is no entity sorter configure, so the order is non-deterministic (fails on PG).

Same with Whats_new_2021_sample_10.

@ajcvickers ajcvickers added this to the 7.0.0 milestone Jan 14, 2022
maumar added a commit that referenced this issue Jan 25, 2022
@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 Jan 25, 2022
maumar added a commit that referenced this issue Jan 26, 2022
@ghost ghost closed this as completed in #27280 Jan 26, 2022
ghost pushed a commit that referenced this issue Jan 26, 2022
@roji
Copy link
Member Author

roji commented Feb 11, 2022

@maumar this is still failing on PG, but this time because of rounding accuracy:

Expected: { Category = Condiments, AveragePrice = 22.308333 }
Actual:   { Category = Condiments, AveragePrice = 22.3083333333333333 }

... because the test implementation rounds on the expected side, presumably to match SQL Server behavior:

public virtual Task Average_Grouped_from_LINQ_101(bool async)
    => AssertQuery(
        async,
        ss => from p in ss.Set<ProductForLinq>()
              group p by p.Category
              into g
              select new { Category = g.Key, AveragePrice = g.Average(p => p.UnitPrice) },
        ss => from p in ss.Set<ProductForLinq>()
              group p by p.Category
              into g
              select new { Category = g.Key, AveragePrice = Math.Round(g.Average(p => p.UnitPrice) - 0.0000005m, 6) },
        elementSorter: e => (e.Category, e.AveragePrice));

I see that InMemory also overrides, probably because of this rounding - should we move the rounding to SQL Server rather than imposing it on all providers by default?

@roji roji reopened this Feb 11, 2022
@ajcvickers ajcvickers removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 6, 2022
maumar added a commit that referenced this issue Aug 10, 2022
Added custom asserter that checks results within range, rather than adding sql server specific expected query

Fixes #27155
@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 Aug 10, 2022
maumar added a commit that referenced this issue Aug 10, 2022
Added custom asserter that checks results within range, rather than adding sql server specific expected query
For the second test, added proper collection asserter so we can impose the order within nested collection.

Fixes #27155
@maumar maumar closed this as completed in 13c2a5f Aug 11, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-rc1 Aug 12, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc1, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
3 participants