There is already an open DataReader associated with this Command without ToList()-CodePudding

I have the method below to load dependent data from navigation property. However, it generates an error. I can remove the error by adding ToList() or ToArray(), but I'd rather not do that for performance reasons. I also cannot set the MARS property in my web.config file because it causes a problem for other classes of the connection.

How can I solve this without using extension methods or editing my web.config?

public override void LoadDependent(IEnumerable<Ques> data)
{
    base.LoadDependent(data);

    if (data.Any())
    {
        foreach (Ques qst in data)
        {
            if (qst?.Id_user != null)
            {
                db.Entry(qst).Reference(q => q.AspNetUsers).Load();
            }
        }
    }
}

CodePudding user response：

I take it from this question you've got a situation something like:

// (outside code)
var query = db.SomeEntity.Wnere(x => x.SomeCondition == someCondition);
LoadDependent(query);

Chances are based on this method it's probably a call stack of various methods that build search expressions and such, but ultimately what gets passed into LoadDependent() is an IQueryable<TEntity>.

Instead if you call:

// (outside code)
var query = db.SomeEntity.Wnere(x => x.SomeCondition == someCondition);
var data = query.ToList();
LoadDependent(data);

Or.. in your LoadDependent changing doing something like:

base.LoadDependent(data);
data = data.ToList();

or better,

foreach (Ques qst in data.ToList())

Then your LoadDependent() call works, but in the first example you get an error that a DataReader is open. This is because your foreach call as-is would be iterating over the IQueryable meaning EF's data reader would be left open so further calls to db, which I'd assume is a module level variable for the DbContext that is injected, cannot be made.

Replacing this:

db.Entry(qst).Reference(q => q.AspNetUsers).Load();

with this:

db.Entry(qst).Reference(q => q.AspNetUsers).LoadAsync();

... does not actually work. This just delegates the load call asynchronously, and without awaiting it, it too would fail, just not raise the exception on the continuation thread.

As mentioned in the comments to your question this is a very poor design choice to handle loading references. You are far, far better off enabling lazy loading and taking the Select n 1 hit if/when a reference is actually needed if you aren't going to implement the initial fetch properly with either eager loading or projection.

Code like this forces a Select n 1 pattern throughout your code.

A good example of loading a "Ques" with it's associated User eager loaded:

var ques = db.Ques
    .Include(x => x.AspNetUsers)
    .Where(x => x.SomeCondition == someCondition)
    .ToList();

Whether "SomeCondition" results in 1 Ques returned or 1000 Ques returned, the data will execute with one query to the DB.

Select n 1 scenarios are bad because in the case where 1000 Ques are returned with a call to fetch dependencies you get:

var ques = db.Ques
    .Where(x => x.SomeCondition == someCondition)
    .ToList(); // 1 query.

foreach(var q in ques)
     db.Entry(q).Reference(x => x.AspNetUsers).Load(); // 1 query x 1000

1001 queries run. This compounds with each reference you want to load.

Which then looks problematic where later code might want to offer pagination such as to take only 25 items where the total record count could run in the 10's of thousands or more. This is where lazy loading would be the lesser of two Select n 1 evils, as with lazy loading you know that AspNetUsers would only be selected if any returned Ques actually referenced it, and only for those Ques that actually reference it. So if the pagination only "touched" 25 rows, Lazy Loading would result in 26 queries. Lazy loading is a trap however as later code changes could inadvertently lead to performance issues appearing in seemingly unrelated areas as new referenences or code changes result in far more references being "touched" and kicking off a query.

If you are going to pursue a LoadDependent() type method then you need to ensure that it is called as late as possible, once you have a known set size to load because you will need to materialize the collection to load related entities with the same DbContext instance. (I.e. after pagination) Trying to work around it using detached instances (AsNoTracking()) or by using a completely new DbContext instance may give you some headway but will invariably lead to more problems later, as you will have a mix of tracked an untracked entities, or worse, entities tracked by different DbContexts depending on how these loaded entities are consumed.

An alternative teams pursue is rather than a LoadReference() type method would be an IncludeReference() type method. The goal here being to build .Include statements into the IQueryable. This can be done two ways, either by magic strings (property names) or by passing in expressions for the references to include. Again this can turn into a bit of a rabbit hole when handling more deeply nested references. (I.e. building .Include().ThenInclude() chains.) This avoids the Select n 1 issue by eager loading the required related data.

CodePudding user response：

I have solved the problem by deletion the method LoadDepend and I have used Include() in my first query of data to show the reference data in navigation property