3

Yesterday, a nice person helped me to build a PredicateBuilder for Linq to Entities here.
Seems to work fine, however the complete query generates this hideous 70 000 lines long thing here (too long to be pasted), and raising SQL statement is nested too deeply.

Here is the context :
The user is looking for a list of animals matching his criteria, notably regarding abilities.
In the GUI, for each ability type (ex: "maniability", "agility" etc.), the user can select a modifier (">", "<", or "=") and a value.
For example, he may want to display "All animals that have an ability potential > 3 in agility", or "All animals that have an ability skill < 10 in maniability and ability potential = 2 in agility"

About the database:

Player with columns Id
Animal with columns Id
Ability with columns :

  • Id
  • AnimalId
  • TypeId (representing Enum which can be "Potential", "BirthPotentiel" or "Skill")
  • AbilityId (representing Enum which can be "Agility", or "Maniability")
  • Value

Thus, each animal has an AllAbilities property which is an ICollection<Ability>.

Here is the search function (all parameters have previously been entered, or left blank, by the user in the GUI).

public async Task<List<Animal>> Search
    (
        Player player,
        int speciesId,
        int breedId,
        int coatId,
        int genderId,
        int minAge,
        int maxAge,
        int priceModifier, // int representing an Enum Criteria.ModifierE: ">", "<" or "="
        int priceValue,
        string ownerPseudo,
        bool isSearchingOwn,
        int minHeight,
        int maxHeight,
        int minWeight,
        int maxWeight,
        List<int> character, // representing list of Enum Flags
        List<int> abilitySkillModifiers, // representing list of Enum ModifierE: ">", "<" or "="
        List<int> abilitySkillValues,
        List<int> abilityPotentialModifiers, // representing list of Enum ModifierE: ">", "<" or "="
        List<int> abilityPotentialValues
    )
    {
        // You can see "PredicateUtils" class following the first link of this post
        var filter = PredicateUtils.Null<Animal>();

        filter = filter.And(e => speciesId != -1 ? e.SpeciesId == speciesId : true);
        filter = filter.And(e => breedId != -1 ? e.BreedId == breedId : true);
        filter = filter.And(e => coatId != -1 ? e.CoatId == coatId : true);
        filter = filter.And(e => genderId != -1 ? e.GenderId == genderId : true);
        filter = filter.And(e => minAge != -1 ? e.age >= minAge : true);
        filter = filter.And(e => maxAge != -1 ? e.age <= maxAge : true);

        string pseudo = isSearchingOwn ? player.Pseudo : ownerPseudo;
        filter = filter.And(e => !string.IsNullOrEmpty(ownerPseudo) ? e.Owner.Pseudo.Equals(pseudo, StringComparison.InvariantCultureIgnoreCase) : true);
        filter = filter.And(e => minHeight > 0 ? e.FinalHeight >= minHeight : true);
        filter = filter.And(e => maxHeight > 0 ? e.FinalHeight <= maxHeight : true);
        filter = filter.And(e => minWeight > 0 ? e.FinalWeight >= minWeight : true);
        filter = filter.And(e => maxWeight > 0 ? e.FinalWeight <= maxWeight : true);
        filter = filter.And(e => character.All(c => (e.character & c) == c));

        for (int i = 0; i < abilitySkillValues.Count; i++)
        {
            filter = filter.And(
                AbilitySkillFilter
                (
                    (Criteria.ModifierE)abilitySkillModifiers[i], // ">", "<", or "="
                    i,
                    abilitySkillValues[i] // value entered by the user for the current ability
                )
            );
        }

        for (int i = 0; i < abilityPotentialValues.Count; i++)
        {
            filter = filter.And(
                AbilityPotentialFilter
                (
                    (Criteria.ModifierE)abilityPotentialModifiers[i], // ">", "<", or "="
                    i,
                    abilityPotentialValues[i] // value entered by the user for the current ability
                )
            );
        }
        return await GetAll(filter);
    }

And the abilities filter functions :

static Expression<Func<Animal, bool>> AbilitySkillFilter(Criteria.ModifierE modifier, int abilityId, int userValue)
    {
        if (modifier == Criteria.ModifierE.More) // User chose ">"
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId).Value >= userValue
                : value <= 0;
        else if (modifier == Criteria.ModifierE.Equal) // User chose "<"
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId).Value == userValue
                : value == 0;
        else if (modifier == Criteria.ModifierE.Less) // User chose "<"
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId).Value <= userValue
                : value >= 0;
        else
            return null;
    }

    static Expression<Func<Animal, bool>> AbilityPotentialFilter(Criteria.ModifierE modifier, int abilityId, int userValue)
    {
        if (modifier == Criteria.ModifierE.More)
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId).Value >= userValue
                : e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId).Value >= userValue;
        else if (modifier == Criteria.ModifierE.Equal)
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId).Value == userValue
                : e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId).Value == userValue;
        else if (modifier == Criteria.ModifierE.Less)
            return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
                ? e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId).Value <= userValue
                : e.AllAbilities.FirstOrDefault(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId).Value <= userValue;
        else
            return null;
    }

Explanation :
In database, Ability rows with TypeId == Potential or TypeId == Skill may not exist, while TypeId == BirthPotential always do.

  • In case TypeId == Potential does not exist for the current animal and the current AbilityId, I want to compare user value with TypeId == BirthPotential row value (which always exists).
  • In case TypeId == Skill does not exist for the current animal and the current AbilityId, I want to compare user value with 0.

If anyone has any suggestion about why this query is producing such an horrible ouput and has an improvement, I would be really grateful. Don't hesitate if you need more information.

SOLUTION:

It finally works, thanks to juharr proposal (using simple if instead of ternary if to not add the clause if not necessary), combined to Ivan Stoev solution.
With criteria on age, gender, species, pseudo, minheight, maxheight, character, one skill ability and one potential ability, here is the new SQL output: almost 70 000 lines to 60 !
Result here

Thanks a lot !

Community
  • 1
  • 1
Flash_Back
  • 565
  • 3
  • 8
  • 31
  • 1
    Is the SQL Server working on the same machine as the application? If so, maybe getting all filtered data (without the filters in the loops) to a local collection and then do the loop-filtering on that is an option. – schlonzo Mar 29 '16 at 12:16
  • 1
    Instead of putting the if condtion into the predicate why not conditionally apply them like `if(speciesId != -1) filter = filter.And(e => e.SpeciesId == speciesId);`. That might help reduce the complexity of the query. – juharr Mar 29 '16 at 12:20
  • @schlonzo : Yes it does. So you would recommend to do `await GetAll(filter)` twice, one right before the loops, and one at the end ? @juharr : Indeed, you're right, I will edit my code this way. However if the user enters every criteria he still won't be able to get results : ( – Flash_Back Mar 29 '16 at 12:25
  • @Flash_Back Even if all the `if`s are true it should still result in less complex query. Though I think the stuff in the loops is more likely the culprit. – juharr Mar 29 '16 at 12:26
  • @juharr Yes, I'm thinking of you getting all the filtered data until the loop-filter part of your program. Store this in a list or something with `ToListAsync()`and then do the rest of the filtering on that list. – schlonzo Mar 29 '16 at 12:31
  • @juharr: Ok, alright then ! I'm currently doing this and will test right away even if I also think that the loops are somehow wrong. If I comment them out every thing seems to be going right. – Flash_Back Mar 29 '16 at 12:31
  • Why not add a custom filter to the Animal class : class Animal : IQueryable – jdweng Mar 29 '16 at 12:34
  • @jdweng What should be changed in the code if I make `Animal` class implement `IQueryable` ? @schlonzo : If I call `GetAll` once I will get the list of animals matching the first criteria. But then how can I tell the two filter functions to search only in animals returned by the first `GetAll` ? Actually, it's written like that `e => e.AllAbilities` but `e` should be restricted to the List previously returned. – Flash_Back Mar 29 '16 at 12:41
  • @Flash_Back Yesterday problem was presented a bit differently. Here your `AllAbilities` filters are overcomplicated, hence generating too much subqueries. – Ivan Stoev Mar 29 '16 at 12:44
  • @Ivan: Oh, so nice to see you there ! :D Thanks again for yesterday the builder is perfect. I actually simplified a lot on yesterday's post because I did not have real reasons to explain all this context to know how to handle the list comparison by index. This time this is the actual and complete context ;) Thus would you also try to call `GetAll` several times to make subqueries ? – Flash_Back Mar 29 '16 at 12:48
  • How can I - don't have your environment :) But currently looking your filters implementation, almost sure it can be trimmed down. – Ivan Stoev Mar 29 '16 at 12:57
  • You can have different GetAll() methods with different parameter lists if types are different or put nulls into parameters that you don't want in search. – jdweng Mar 29 '16 at 13:24

1 Answers1

2

When doing dynamic filtering, try to do more static evaluation outside of the expressions. This way you'll get better queries because currently EF does not optimize constant expressions except for building static IN (...) list criteria.

But the main problem with your current code are the usages of FirstOrDefault inside your ability filters. In general try to avoid using any query construct type that can lead to SQL subquery, because as you saw, for some reason EF is nesting all the subqueries, hence you get that monstrosity SQL and the error. The safe construct is to use Any which is translated to SQL EXISTS subquery w/o nesting.

So try this and see what you'll get:

static Expression<Func<Animal, bool>> AbilitySkillFilter(Criteria.ModifierE modifier, int abilityId, int userValue)
{
    Expression<Func<GameAnimal, bool>> filter = null;
    bool includeMissing = false;
    if (modifier == Criteria.ModifierE.More) // User chose ">"
    {
        filter = e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId && a.Value >= userValue);
        includeMissing = userValue <= 0;
    }
    else if (modifier == Criteria.ModifierE.Equal) // User chose "="
    {
        filter = e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId && a.Value == userValue);
        includeMissing = userValue == 0;
    }
    else if (modifier == Criteria.ModifierE.Less) // User chose "<"
    {
        filter = e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId && a.Value >= userValue);
        includeMissing = userValue >= 0;
    }
    if (filter != null && includeMissing)
        filter = filter.Or(e => !e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Skill && a.AbilityId == abilityId));
    return filter;
}

static Expression<Func<Animal, bool>> AbilityPotentialFilter(Criteria.ModifierE modifier, int abilityId, int userValue)
{
    if (modifier == Criteria.ModifierE.More)
        return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
            ? e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId && a.Value >= userValue)
            : e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId && a.Value >= userValue);
    else if (modifier == Criteria.ModifierE.Equal)
        return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
            ? e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId && a.Value == userValue)
            : e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId && a.Value == userValue);
    else if (modifier == Criteria.ModifierE.Less)
        return e => e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId)
            ? e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.Potential && a.AbilityId == abilityId && a.Value <= userValue)
            : e.AllAbilities.Any(a => a.TypeId == (int)Ability.TypeE.BirthPotential && a.AbilityId == abilityId && a.Value <= userValue);
    else
        return null;
}
Ivan Stoev
  • 195,425
  • 15
  • 312
  • 343
  • 2
    You probably can't imagine how thankful I am (again) ! Not only it works perfectly fine, but I also understood what was actually going on and will be able to use this base everywhere in my project. I edited my first post with the new SQL output, 70 000 lines to only 60, incredible ! Thanks again to have taken time to help me, hoped I could do more than just accepting the answer ;) – Flash_Back Mar 29 '16 at 15:00
  • 1
    You are welcome, glad that helped:) Good luck with your project, cheers! – Ivan Stoev Mar 29 '16 at 15:20