3

In 2.2 I could write something like this:

        List<Claim> claims = new List<Claim>();
        var userRoles = await _userManager.GetRolesAsync(user);
        foreach (var role in _roleManager.Roles.Where(a => userRoles.Contains(a.Name)))
        {
            claims.AddRange(await _roleManager.GetClaimsAsync(role));
        }

        return claims;

In 3.1 it gives me this error:

System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.

But if I add

ToList()

to the forEach clause, it works fine(like this):

        List<Claim> claims = new List<Claim>();
        var userRoles = await _userManager.GetRolesAsync(user);
        foreach (var role in _roleManager.Roles.Where(a => userRoles.Contains(a.Name)).ToList())
        {
            claims.AddRange(await _roleManager.GetClaimsAsync(role));
        }

        return claims;

Should I change all places in my code where I used similar construct, or there is a way to make EF work normal with it?

Jamil
  • 830
  • 2
  • 15
  • 34
  • Does this answer your question? [There is already an open DataReader associated with this Command which must be closed first](https://stackoverflow.com/questions/6062192/there-is-already-an-open-datareader-associated-with-this-command-which-must-be-c) – Max Jan 10 '20 at 08:11
  • I read that question, and yes, technically it solves my problem. I just want to know, why this wasn't an issue in 2.2 and became an issue in 3.1. – Jamil Jan 10 '20 at 08:16
  • @Jamil Have you also changed the framework, e.g. from .NET Framework to NET Core? I mean, it's possible that the issue has nothing to do with EF Core, but the underlying data provider MARS default value. – Ivan Stoev Jan 10 '20 at 08:29
  • @Ivan, Target framework was .NET Core 2.2, I changed it to 3.1(hoped it will solve "reducible node" error) – Jamil Jan 10 '20 at 08:36
  • Nevermind. It's really caused by EF Core code - the new rewritten query processing pipeline. – Ivan Stoev Jan 10 '20 at 09:00
  • @IvanStoev how so ? MARS should allow this. Does the new pipeline avoid MARS? Have you found a specific file in Github? Asking out of curiosity – Panagiotis Kanavos Jan 10 '20 at 09:46
  • @PanagiotisKanavos It's the opposite. Looks like the old pipeline avoids MARS, while the new one doesn't. I haven't searched the EF Core GitHub issues, just ran similar test (iteration over query results and executing another query inside the loop) in 2.2 and 3.1, and the detailed log showed that the 2.2 is closing the reader before executing the second query. Btw, default MARS setting of the connection in both cases is `false`, so in 3.x it has to be set explicitly to `true` inside connection string. – Ivan Stoev Jan 10 '20 at 10:07
  • @IvanStoev If that's correct it's a massively significant breaking change that hasn't been documented **anywhere**. – Ian Kemp Jan 10 '20 at 10:15
  • @IvanStoev `the 2.2 is closing the reader before executing the second query` that would require *eagerly* reading all results and caching them contrary to expectations. That would be rather ... unfortunate in an export scenario and definitely not the expected behavior. Didn't EF 6.2 use MARS? – Panagiotis Kanavos Jan 10 '20 at 10:17
  • @IanKemp a breaking change in *2.2* - instead of enumerating the results as they come, it has to cache all of them before closing the reader .... Oh wait, [the change came in EF 6.2](https://stackoverflow.com/questions/47208528/entity-framework-upgrade-to-6-2-0-from-6-1-x-breaks-certain-queries-unless-i-ena) ??? – Panagiotis Kanavos Jan 10 '20 at 10:21
  • @PanagiotisKanavos Indeed, I guess they were fully materializing and buffering it internally. Which btw with *tracked* queries is not that unexpected, since they would be added to the change tracker internal lists anyway. I found also that 2.x no tracking queries throw runtime exception, although not related to MARS. Shortly, it's a mess :) – Ivan Stoev Jan 10 '20 at 10:21
  • @IvanStoev check [this question about 6.2](https://stackoverflow.com/questions/47208528/entity-framework-upgrade-to-6-2-0-from-6-1-x-breaks-certain-queries-unless-i-ena). Looks like the change came in 6.2, then EF Core 1-2 used various tricks to cover for missing functionality, with unintended (or unfortunate) consequences – Panagiotis Kanavos Jan 10 '20 at 10:24

2 Answers2

1

why ToList() works?

  • ToList() forces execution on select query.

why it throw expcetion without adding ToList()?

  • It will throw the exception if you dont force execution due to the query actually executed when the query variable is iterated over not when select variable is created

Updates

older version of EF before 3.0 cannot convert expression to SQL or parameter and it evaluates the LINQ expression in client side automatically however new version of EF allow expression in last select call in query, so if expression in any other part of the query cannot be converted to SQL or parameter then it throws exception

Source Microsoft Docs, can read more here

Saif
  • 2,611
  • 3
  • 17
  • 37
  • 1
    I don't get why it was working in 2.2, and now it throws an exception. "It will throw the exception if you dont force execution due to the query actually executed when the query variable is iterated over not when select variable is created." - pretty sure I used deferred execution with ef core and it didn't throw any exceptions before – Jamil Jan 10 '20 at 08:24
  • 3
    @Gonzo345 Really? OP code is *iterating over*, hence the query *is executed*. So the issue has nothing to do with deferred execution. – Ivan Stoev Jan 10 '20 at 08:26
  • 1
    @Saif this isn't about client-side evaluation. What the OP did should work - `foreach` iterates over the results of one query and inside the loop runs another query. Using MARS (the default) should allow this. – Panagiotis Kanavos Jan 10 '20 at 09:41
  • +1. I'm a little fuzzy on exactly what changed between EF 2.2 and EF 3.0, but I have found that selectively materializing my Linq expressions seems to solve the new errors that have sprung up in my application. – John M Gant Jan 23 '20 at 19:15
  • What if it fails the same when when using ToList? – Captain Prinny Feb 11 '21 at 14:52
1

becasue_roleManager.Roles.Where(a => userRoles.Contains(a.Name) creates an IEnumerator which keeps the connection open, when it try to

do await _roleManager.GetClaimsAsync(role) again.. it see that it already has a open DataReader associated

3.0 probably did a internal evaluates the LINQ expression in client side automatically kind of like an auto Tolist(), instead of IEnumerator which keeps the connection open.

To avoid this i would do below to all future and current code. That way it doesn't matter, also ensure not executing over IEnumerator statements which re-execute the entire statement.

Further this has the added benefit that you can debug the list before the loop.

Before someone says that this consume more mem, show me a test showing so... This does not use yield and also the compiler is probably smart enough to make optimization for anything it sees as not been needed.

Here they word the IEnumerator issue better but same same.

Entity Framework upgrade to 6.2.0 from 6.1.x breaks certain queries unless I enable MARS

List<Claim> claims = new List<Claim>();
var userRoles = await _userManager.GetRolesAsync(user);
//you could change this to use async and await
//var roles = await _roleManager.Roles.Where(a => userRoles.Contains(a.Name)).ToListAsync();
var roles = _roleManager.Roles.Where(a => userRoles.Contains(a.Name)).ToList();
foreach (var role in roles)
{
    claims.AddRange(await _roleManager.GetClaimsAsync(role));
}

return claims;
Seabizkit
  • 2,417
  • 2
  • 15
  • 32