An Entity Framework expression to find entities that don't match some property

Question

I need a custom expression which works in Entity Framework. The method should have this signature:

var ids = new List<int> { 1, 2, 3 };
Context.FooEntities.WithoutId(e => e.Id, ids);

That should give me all Foo entities which do not have Id properties that match those in the list.

My attempt is based on an existing example here.

public static IQueryable<T> WithoutId<T>(
    this IQueryable<T> entities,
    Expression<Func<T, int>> propertySelector,
    ICollection<int> ids) {

    var property = (PropertyInfo)((MemberExpression)propertySelector.Body).Member;

    ParameterExpression parameter = Expression.Parameter(typeof(T));

    var expression = Expression.Lambda<Func<T, bool>>(
        Expression.Not(
            Expression.Call(
                Expression.Constant(ids),
                typeof(ICollection<int>).GetMethod("Contains"), 
                Expression.Property(parameter, property))), 
        parameter);

    return entities.Where(expression);
}

The problem is when ids is empty then it returns all entities. It should return no entities.

Strange requirement. You want to exclude some ids, and if there are no ids to be excluded, it's normal to not exclude anything, i.e. include everything, i.e. return all entities. — Ivan Stoev, Sep 12 '16 at 15:28
@IvanStoev No, it should NOT return any entities. It should return an empty set. — h bob, Sep 12 '16 at 15:46
@hbob - I think he gets that, he's saying that it's an illogical operation. — Erik Funkenbusch, Sep 12 '16 at 16:03
@ErikFunkenbusch Maybe you're right... though can you describe in formal logic why it's illogical? I can't remember exactly how to do that, but it seems that it's not an issue of logic but rather expectation: "give me all entities whose Ids are not {nothing}". Does that mean they must be "something"? Maybe it is formally illogical, I don't know, my head hurts thinking about it :-) — h bob, Sep 12 '16 at 16:13
@hbob - In set theory (which is what Link and SQL are based on) "nothing" is called the "Empty Set". It's like an empty collection. So yes, saying that something does not include the empty set means return everything (other than an empty set, which is what you are looking to return, so it's the opposite of the logical expectation) — Erik Funkenbusch, Sep 12 '16 at 16:24
@hbob - Also, how can you tell the difference between the situation where all items are in the set? Using your logic, all = none and none = none — Erik Funkenbusch, Sep 12 '16 at 17:27

score 1 · Accepted Answer · answered Sep 12 '16 at 15:06

1

How about this? (just as idea, not completed code)

IEnumerable<Entity> Get()
{
    var ids = new[] { 1, 2, 3 };
    if (ids.Length == 0) return Enumerable.Empty<Entity>();

    return MyContext.MyEntities.Where(x=>ids.Contains(x.Id)).ToArray();
}

answered Sep 12 '16 at 15:06

tym32167

4,741
2
28
32

No it needs to be runnable as an expression that EF would be able to handle. This avoids the call altogether, but that is not always possible, hence the requirement. – h bob Sep 12 '16 at 15:47
But EF able to handle this ```x=>ids.Contains(x.Id)```. Or I don't understand your question (just my bad english :)). – tym32167 Sep 12 '16 at 15:55
The "where+contains" is the right way, and avoids all the expressions... – h bob Sep 12 '16 at 16:07
@hbob The premise of your question was that you needed a general purpose method to do this for any type of queryable. If you don't actually need that, then you shouldn't ask for it in your question. – Servy Sep 12 '16 at 16:10
@Servy Erik Philips' answer shows that the question is asking for something that can be done using a much simpler and built-in approach. So although the question was valid, it is "overkill" to answer it. That's why I chose this answer. If I am wrong, please propose which answer is correct, as you yourself discovered that that other answer won't work? – h bob Sep 12 '16 at 16:15
@Servy I added a formal approach below, what do you think of it? – h bob Sep 12 '16 at 16:19
1

@Servy, disagree. Initial question was about "That should give me all Foo entities which do not have Id properties that match those in the list." While other answers tried to improve suggested solution, I tried to solve initial problem. – tym32167 Sep 12 '16 at 16:26
1

@hbob Eric's answer doesn't even *compile*. It's nowhere near a working answer. This answer in no way meets the requirements of the question, as you stated them. It doesn't implement the method with the signature specified as the requirements. – Servy Sep 12 '16 at 16:54
1

@tym32167 The question *specifically* mentioned that it needed to create a method with a particular signature where the column being filtered on is specified in an `Expression`. This doesn't do that, and thus doesn't meet the requirements of the question. – Servy Sep 12 '16 at 16:55
@Servy Exactly. I see no any logical reason (although the same can be said for the question) of why this "answer" is marked as accepted. – Ivan Stoev Sep 12 '16 at 16:59
@tym32167 You're the man. Thanks for being helpful. Thanks also to IvanStoev, ErikFunkenbusch, octavioccl, Sampath and ErikPhilips who all took time out from their busy day in order to help some stranger on the Internet. I know more now because of you guys. Thanks. – h bob Sep 12 '16 at 17:01
If Servy cares so much, he should have added his own answer instead of nit picking everyone elses answers. – h bob Sep 12 '16 at 17:04
@IvanStoev Apparently he [intentionally marked an answer he knows is incorrect](http://stackoverflow.com/questions/39453294/an-entity-framework-expression-to-find-entities-that-dont-match-some-property/39454679#comment66231659_39454679) just to spite me for pointing out that it's not a correct answer. – Servy Sep 12 '16 at 17:09
@hbob I think you shouldn't be offensive to Servy. I'm pretty sure he could easily answer the question, but he cares about quality of the posted answers, which I think is good for the site. In fact IMO all the answers are wrong - no general usage library author will use such things in a generic method, because all those `IEnumerable` to `IQueryable` variations change the context of the query, so if it's combined with `DefaultIfEmpty` and other EF specific conditions (like canonical functions) it would lead to runtime exceptions... – Ivan Stoev Sep 12 '16 at 17:50
@hbob ... Since the behavior you need is exceptional case, the simplest and definitely not "overkill" solution is to insert at the beginning of the method something like this `if (!ids.Any()) return entities.Where(_ => false);` and keep the rest of the method as it is. – Ivan Stoev Sep 12 '16 at 17:53
1

@Servy , "This answer in no way meets the requirements ... It doesn't implement the method". I dont have to implement exactly that method to give useful answer. This is not obligatory. I give answer that helps user, which asked. This is main purpose of this website, to give useful answers to people. For me is obvious, what bob wants to ask and I give answer in couple of minutes. If you dont agree with this, so, sorry. – tym32167 Sep 12 '16 at 20:34
@tym32167 When the question *specifically* says that they need something, and you give them something radically different that *doesn't accomplish what they asked for* then you have posted an incorrect answer. That you don't care that you've posted a completely incorrect answer that's not actually helping to solve the problem just because it would take you more than the very short period of time it took you to write what you did is rather unfortunate. – Servy Sep 12 '16 at 20:44
1

@Servy I still dont agree, I think, that my answer is very match to question and its correct. I dont have to implement exactly that method, but I show just Idea, how that method can be implemented. I dont see here any "radically different" issues. If you think that only implementation of that method can be as correct answer, please, prove it with some point from website rules or some other documents, because for me it still is not clear. – tym32167 Sep 12 '16 at 20:56
@tym32167 You hard coded the query, rather than allowing it to work for *any* query, as the question specifies, and you hard code the field, rather than writing it so that it works for *any* field, as the question specifies. Those are pretty radical changes, to say that they don't matter, when the question *explicitly states that they do* is clearly wrong. That's the only signature a correct implementation can have because the question *defined* that as being a requirement, thus any answer meeting all of the requirements in the question must meet that requirement. – Servy Sep 12 '16 at 21:01
1

@Servy yes, I hardcoded query and field accessor. In words, my answer looks like "you dont need expressions that you trying use". I dont see any reason to add into my code more complexity, but say same. And, moreover, as i understand from question, author does not have any problems with method signature. So, why should I worry about that signature? Yes, my answer is not full, but it enough to help bob solve his problem, right? Again, I agree, that my answer is not full, but I disagree that it Incorrect or radically different from question. – tym32167 Sep 12 '16 at 21:22
Why would you assume that he doesn't need it when *he specifically said that he needs it*. If he didn't need it then *he wouldn't have said that it was necessary*. It's a *fact* that your answer doesn't meet the requirements of the question. It's not a matter of *opinion that you can agree or disagree with. The question is very explicit about its requirements, and you unquestionably don't meet those requirements. That you don't *care* that your answer is incorrect and not what the question is asking doesn't mean that it isn't those things. – Servy Sep 12 '16 at 21:25
1

@Servy Nothing can not be fact just because you said so. Take a look to question title, which should contain essential of question. The main question was about expression. This is required for answer. Not function implementation, but expression. Anyway, I feeling that we are stuck here. Thank for discussion. I very respect your achievement here, but I still dont understand why I have to follow exactly all question requirements instead of just helping people to solve their programming problems. But, I also dont see any reason to continue discussion. Good luck and happy programming :) – tym32167 Sep 12 '16 at 21:40
@tym32167 Considering you have *also* said that your answer doesn't answer the question, it is apparently an undisputed claim. Indeed it's not true because I say so, but it is true, and you have said as much yourself. That you don't *care* that you aren't answering the question is unfortunate, but it is the case. Like I said, I can't force you to care about not answering the question, I can only point out that you're not. That you're satisfied posting incorrect answers that aren't helpful is unfortunate, but that you're uninterested in providing useful information is your decision. – Servy Sep 12 '16 at 21:44
@Servy It is my question, and I decide which answer I like. And I like the answer from tym32167. Relax and stop being combative. – h bob Sep 16 '16 at 08:15
@hbob You're correct that nobody can stop you from accepting an answer you and the author both know is incorrect just to be mean to other people. – Servy Sep 16 '16 at 13:08
@Servy I like his answer, and his answer helped me solve my problem. The fact that you hate it more than any of the other answers, is another matter. – h bob Sep 17 '16 at 10:06

ocuenca · Answer 2 · 2016-09-12T16:07:14.717

In case the list of ids is empty, just return an empty collection:

if (ids.Count() != 0)
{
   var property = (PropertyInfo)((MemberExpression)propertySelector.Body).Member;

    ParameterExpression parameter = Expression.Parameter(typeof(T));

    var expression = Expression.Lambda<Func<T, bool>>(
        Expression.Not(
            Expression.Call(
                Expression.Constant(ids),
                typeof(ICollection<int>).GetMethod("Contains"), 
                Expression.Property(parameter, property))), 
        parameter);

    return entities.Where(expression);
}
return new List<T>().AsQueryable()//Or Enumerable.Empty<T>().AsQueryable();

Sampath · Answer 3 · 2016-09-12T16:17:02.220

You can try as shown below.

public static IQueryable<T> WithoutId<T>(this IQueryable<T> entities,Expression<Func<T, int>> propertySelector,ICollection<int> ids) {

    if (ids.Any())
     {
    var property = (PropertyInfo)((MemberExpression)propertySelector.Body).Member;

    ParameterExpression parameter = Expression.Parameter(typeof(T));

    var expression = Expression.Lambda<Func<T, bool>>(
        Expression.Not(
            Expression.Call(
                Expression.Constant(ids),
                typeof(ICollection<int>).GetMethod("Contains"), 
                Expression.Property(parameter, property))),parameter);

    return entities.Where(expression);
   }
   else{
      return Enumerable.Empty<T>().AsQueryable();
     }
}

Returning null would be very unexpected to the caller. He would assume results, or an empty set. — h bob, Sep 12 '16 at 15:49

h bob · Answer 4 · 2016-09-12T16:57:49.263

0

My question can be solved without a complicated expression, as indicated in the above answers (using a simple "where+contains", which is supported by EF).

But, the formal way could be this, which seems to work for me (even though it's overkill):

public static IQueryable<T> WithoutId<T>(
    this IQueryable<T> entities,
    Expression<Func<T, int>> propertySelector,
    ICollection<int> ids) {

    if (!ids.Any()) {                                          // here is the trick
        /*
        expression = Expression.Lambda<Func<TEntity, bool>>(
            Expression.Constant(false), 
            parameter);
        */
        return Enumerable.Empty<T>().AsQueryable()
    }

    var property = (PropertyInfo)((MemberExpression)propertySelector.Body).Member;

    ParameterExpression parameter = Expression.Parameter(typeof(T));

    var expression = Expression.Lambda<Func<T, bool>>(
        Expression.Not(
            Expression.Call(
                Expression.Constant(ids),
                typeof(ICollection<int>).GetMethod("Contains"), 
                Expression.Property(parameter, property))), 
        parameter);

    return entities.Where(expression);
}

edited Sep 12 '16 at 16:57

answered Sep 12 '16 at 16:18

h bob

3,610
3
35
51

Note: 1) this is overkill as EF supports "where+contains", 2) as @ErikFunkenbusch commented the existing behavior (returning everything rather than nothing) is correct. That means that this is the correct technical answer to the question, but it shouldn't be used. – h bob Sep 12 '16 at 16:49
There is no value gained by calling `Where` with a predicate that is always false over just returning an empty queryable, as shown in other answers. And even if you *did* want to do that, there's no reason to build the expression manually over using a lambda. When actually doing the filtering is another matter of course. – Servy Sep 12 '16 at 16:51
Of course EF supports calling `Contains` in a `Where`. The expression code form the question *is building an expression that calls `Contains`*. And building the expression *is* of course necessary if you want to use the property selector that you have to apply this to a field not known when writing this method. – Servy Sep 12 '16 at 16:52
@Servy Edited. Better? – h bob Sep 12 '16 at 16:55
So now you're just duplicating what two other answers have done. You shouldn't be posting a new answer just to copy another answer, but rather because you have a new solution to add. – Servy Sep 12 '16 at 16:56
@Servy I've bent over backwards trying to get you to approve, but I'm not sure why. I've added edits on top of edits, which is why it appears similar now. The other guys in this thread were genuinely helpful. You just wasted my time. I've marked the answer you least like. Good bye. – h bob Sep 12 '16 at 16:59
I pointed out *one* issue with your answer, not many; you made *one* edit in response to that one point. You spent like a minute making that edit. How is that wasting lots of time? That you would go out of your way to accept an answer you realize is bad just to spite me for pointing out why someone else's answer doesn't correctly answer the question is, rather odd. Why you wouldn't just use one of the correct answers that propose what you've shown in this answer here do what you want is beyond me. – Servy Sep 12 '16 at 17:04
@Servy you made criticisms on every comment in every answer, and I replied to all of them, not just here. Post you own brilliant answer then. I accepted it because it works for me, and is better than a technical answer which is illogical. ErikFunkenbusch and IvanStoev already pointed out that the "correct" solution which you are pursuing is logically invalid. Future readers should not use it. The accepted answer is simpler and correct, and solves an inherent logical inconsistency in my question, which I owned up to. I came here to learn. – h bob Sep 12 '16 at 17:09
No, I didn't comment on every answer. I commented on most of them, yes. All that had problems requiring correction. Why would I post another answer when there are already 2 perfectly fine answers? LIke I told you, duplicating another answer isn't valuable. The answer you accepted *doesn't solve the problem you asked to solve*, not even close. If you actually have a *different* problem, then why did you ask for what you did? – Servy Sep 12 '16 at 17:14
As for the comments on the question itself, they're essentially arguing that *you should just be using the code form the quesiton*, and that it *already does what it should*. The answer you accepted *doesn't* function in the way those comments on the quesiton were inquiring about. The answer returns an empty set when the list of IDs is empty, rather than the original question, as those comments indicate would be what the name of the method implies should happen. – Servy Sep 12 '16 at 17:16

An Entity Framework expression to find entities that don't match some property

4 Answers4