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

Filters in combination with batch update in EntityFramework.Extended #58

Open
RubenDelange opened this issue Mar 2, 2016 · 7 comments

Comments

@RubenDelange
Copy link

First of all: thank you for this package, really nice!

I'm using the soft delete filter functionality, but I'm currently stuck on an exception that's being thrown when using the .Update() filter of EntityFramework.Extended (batch update functionality)

The error that comes back is System.Data.SqlClient.SqlException: Must declare the scalar variable "@DynamicFilterParam_1".

Code sample:

using (var context = new EntityFrameworkEntitiesDbContext("name=MyEntities"))
{
    //disable soft delete filter first
    context.DisableAllFilters();

    //this works if the filter is querying for T.Deleted = true => x is not null
    var x = await context.Set<T>().Where(filter).FirstOrDefaultAsync();

    //this blows up
    updatedRows = await context.Set<T>().Where(filter).UpdateAsync(updateExpression);
}

It seems like the WHERE (([Extent1].[Deleted] = @DynamicFilterParam_1) OR (@DynamicFilterParam_2 IS NOT NULL)) is still present in the IQueryable after disabling the filter which might explain it crashing in .UpdateAsync()

Any ideas or workarounds would be helpful :) - thanks in advance!

@RubenDelange
Copy link
Author

FYI: not disabling the soft deleted filter before batch updating doesn't help either. At first I wasn't thinking about disabling any filter or instantiating a temporarily new DbContext in the BatchUpdate() function, until my batch update logic broke with the SqlException described above. At that point, I thought disabling all filters within a new using(var context = NewContext()) block might be a solution. Unfortunately it still breaks with the same SqlException :(

@jcachat
Copy link
Collaborator

jcachat commented Mar 3, 2016

These 2 libraries are not going to be compatible as they are.

The reason is that DynamicFilters changes the EF query when it's first executed. It adds the conditions you see for the @DynamicFilterParam_1 parameters into that query. But, the actual parameter values are injected by modifying the raw SQL query after EF creates it and just before it's executed against the database. At that time, we find the current values for those parameters and replace the "@DynamicFilterParam_1" type stuff with the actual values. That is the only way to dynamically adjust the parameter values. If we were to create the parameters into the EF query, the values would be whatever the current value is at the time the query is first created - and then we could not change them again, ever.

EntityFramework.Extended is (and I don't know all the details - I just browsed the code real quick) somehow grabbing the EF query and then turning a select query into a delete, update, etc. It looks like it's sucking in the query with the added DynamicFilter conditions. But, it obviously has no idea how to find & replace the parameter values and these parameters are not actually in the query's parameter collection anyway. Then it executes the resulting query directly so our IDbCommandInterceptor is not getting a chance to do our parameter substitution.

And, as you have seen, disabling the filters will not help. Once EF compiles the query, those conditions are stuck in it for the life of the application (for that specific query footprint). Since it's expensive to compile the query, EF only does it once. So we have to add our conditions that first time it's compiled even if the filter is disabled. It is actually disabled by setting one of the parameters to false in the raw sql each time the query is executed. And there is no way to recompile the query without the conditions once it's been compiled.

Now, with all that said... It might be possible to make things work. But, it would require EntityFramework.Extended exposing some way for us to modify their query immediately before it was executed. Like I'm doing w/EF in my DynamicFilterCommandInterceptor.cs.

@RubenDelange
Copy link
Author

Hi jcachat!

Thank you for your quick and elaborated reply! I was afraid that "the 2 libraries are not compatible" would be the outcome of this so I'm replacing the .UpdateAsync() extension with good old Database.ExecuteSqlCommandAsync() - ADO style ;-)

Should I file your suggestion as a possible improvement on the EntityFramework.Extended github page?

@jcachat
Copy link
Collaborator

jcachat commented Mar 3, 2016

Sure, if they are willing to expose something, I would be happy to work with them on it. I'll leave this issue open so you can refer to it.

@davidnemeti
Copy link

davidnemeti commented Jun 5, 2016

@jcachat, you could override Entity Framework's query plan caching mechanism, so it would be dependent on the used filter parameters. Thus, you could use the filter parameters (accessible through DbContext instances) inside the command tree interceptor, and could even fully omit the generated filter from the generated SQL if it is disabled for that DbContext.

I have implemented a soft delete command tree interceptor myself (and a much more complex interceptor as well, where the generated command tree filter can be totally different depending on the extra parameters), and I am overriding the query plan cache, so it works properly.

You can override the query plan cache in two different ways (you only need to use one of them).

1. The derived DbContext implements IDbModelCacheKeyProvider:

public class MyDbContext : DbContext, IDbModelCacheKeyProvider
{
    // ...

    public string CacheKey => $"{IsEnabledLogicalDeletionFilter}";
}

2. Implement IDbModelCacheKey in a separate class:

public class DbModelCacheKey : IDbModelCacheKey, IEquatable<DbModelCacheKey>
{
    private readonly DbContext _dbContext;

    public DbModelCacheKey(DbContext dbContext)
    {
        _dbContext = dbContext;
    }

    public bool Equals(DbModelCacheKey that) => this.GetEquivalencyItems().SequenceEqualNullSafe(that.GetEquivalencyItems());
    public override bool Equals(object that) => Equals(that as DbModelCacheKey);
    public override int GetHashCode() => GetEquivalencyItems().GetHashCodeSequence();

    private IEnumerable<object> GetEquivalencyItems()
    {
        yield return _dbContext.GetType();

        var myDbContext = _dbContext as MyDbContext;
        if (myDbContext != null)
        {
            yield return myDbContext.IsEnabledLogicalDeletionFilter;
       }
    }
}

and register it in the derived DbConfiguration's constructor:

SetModelCacheKey(dbContext => new DbModelCacheKey(dbContext));

@jcachat
Copy link
Collaborator

jcachat commented Jun 5, 2016

What you are describing with IDbModelCacheKeyProvider will cause the entire DbContext (models and all) to be cached differently inside EF depending on IsEnabledLogicalDeletionFilter. I believe that means any time IsEnabledLogicalDeletionFilter changes, EF will then cache that DbContext based on a new key? I'm also not sure what affect it would have on EF if you dynamically change that after models & queries have already been cached.

I can see how that could work in your case if you only have the 1 soft delete filter and don't change it often. But, I don't think that would be a good approach if you have several filters and a large model. Wouldn't that lead to EF maintaining multiple caches of the entire model? And re-caching/compiling all queries every time the CacheKey changes?

I already use the command tree interceptor. See DynamicFilterInterceptor.cs in the source. This library allows the parameters of the filter to change dynamically (in addition to enable/disable dynamically). When we do that, IDbCommandTreeInterceptor is not called again if the query has already been cached so we do not have the opportunity to re-create the query or to inject the new parameter values. Attempting to inject the parameters during the query caching will result in the issue described in #25 - please see that issue and the link in my response for the full history. It is the reason this library was created in the first place. And if we changed the CacheKey every time a parameter changed, it would constantly cause ALL queries to be re-cached which would lead to memory and performance problems. This would cause huge problems if your filters were being used for multi tenancy, for example - every user would effectively have their own dedicated EF cache!

@Jared314
Copy link

Jared314 commented Sep 7, 2016

I mentioned this in loresoft/EntityFramework.Extended/issues/199, but I will reference it here also.

While not directly applicable to the EntityFramework.Extended batch update, I was able to work around my issues with using EntityFramework.Extended.Future and EntityFramework.DynamicFilters.

The main issues I ran into were:

  1. EntityFramework.Extended.Future.FutureQueryBase using ObjectQuery.ToTraceString() to store the generated sql while only preserving the query parameters that existed before the EntityFramework.DynamicFilters.DynamicFilterInterceptor runs.
  2. EntityFramework.Extended.Future.FutureRunner prefixing all query parameters with its own prefix, f0_DynamicFilterParam_000001 instead of the expected DynamicFilterParam_000001

The first issue could only be solved by modifying EntityFramework.Extended. The second issue, however, required modifying DynamicFilterDefinition.GetFilterAndParamFromDBParameter to be more relaxed.

public static Tuple<string, string, DataSpace> GetFilterAndParamFromDBParameter(string dbParameter)
{
    if (!dbParameter.Contains(DynamicFilterConstants.PARAMETER_NAME_PREFIX))
        return null;

    var parts = dbParameter.Split(new string[] { DynamicFilterConstants.DELIMETER }, StringSplitOptions.None);
    if (parts.Length < 2)
        throw new ApplicationException(string.Format("Invalid format for Dynamic Filter parameter name: {0}", dbParameter));

    int dbParamIndex;
    if (!int.TryParse(parts.Last(), out dbParamIndex))
        throw new ApplicationException(string.Format("Unable to parse {0} as int", parts[1]));

    Tuple<string, string, DataSpace> filterParamKey;
    if (!_ParamIndexToFilterAndParam.TryGetValue(dbParamIndex, out filterParamKey))
        throw new ApplicationException(string.Format("Param {0} not found in _ParamIndexToFilterAndParam", dbParamIndex));

    return filterParamKey;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants