[CSHARP-605] LINQ should translate false bool expressions to {a : false} rather than {a : {$ne : true}} Created: 17/Oct/12 Updated: 22/Jun/22 Resolved: 17/Oct/12 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.6 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Minor - P4 |
| Reporter: | Zaid Masud | Assignee: | Robert Stam |
| Resolution: | Done | Votes: | 0 |
| Labels: | LINQ | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
The following LINQ Expression:
Translates to {BoolField : {$ne : true.}} Well, this is not great for performance and the expression is not able to use an index as well as {BoolField : false}would be able to use the index. Since in the driver serialization, we know that the field will always be serialized to bool, it would be better to translate the expression to {BoolField : false}instead. (Note that (d => d.BoolField == false) appears to translate correctly.) |
| Comments |
| Comment by Bouke Haarsma [ 21/Aug/19 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
On MongoDB 3.2 the query performance is significantly impacted by this decision. `{$ne: true}` on an indexed field does a IXSCAN followed by a FETCH to filter out the `true` values. While it can be argued that it's weird that the index isn't sufficient, the fact is that the driver translation is causing the query in the first place. To me at leat the translation to `{a: false}` would feel more natural. It looks like this query would perform equally well since 3.5.9 (https://jira.mongodb.org/browse/SERVER-27822), as no longer a FETCH will be performed. | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Zaid Masud [ 24/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Yes please close | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 24/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Ok, I think your assessment is well said. I think a slight clarification is in order for those reading this later. We didn't choose #2 to be the same as #4 arbitrarily. We chose for #2 to be exactly the opposite of #1. #4 happens to be the solution that guarantees that. Anyways, given that, can we close this issue? | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Zaid Masud [ 24/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Just to summarize this discussion, we are considering the following boolean expressions:
The last 4 have "natural" translations into mongo query syntax. The first 2 are not as natural. It's subjective whether we consider #2 the same as #3 or the same as #4. It seems like a coin flip to me, so your decision to treat #2 the same as #4 is good enough in my view (as I don't have a stronger argument to say that #2 is closer to #3). | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
I would say several things:
Keep in mind that if all the x.b values in your collection are in fact boolean values, you are going to get the results you are expecting anyway... just that we're not rewriting your queries behind your back. | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Ah, I see your point. Perhaps we should translate x == false to { x : { "$ne" : true }}. If we are really attempting to preserver C# semantics, then you are absolutely right. I wonder what rstam would say about that... | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Zaid Masud [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Actually thinking about this further my understanding here may be incorrect, as I could see !x being semantically equivalent to x != true (rather than x == false). I'll look at the language spec in more detail to confirm. | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Zaid Masud [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Thanks Craig, I don't think this is a major issue at all and this discussion is probably mostly academic at this point. I suppose my only point was that !x and x == false should translate to the same thing (whether that thing is x: {$ne:true} or x : false). If I hadn't seen the implementation and used Robert's transcript, I would've expected the same consistent results for !x and x == false, whatever that result was. But I understand there are two sides to the argument and see your point as well. | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
Yes, semantically equivalent in C#. Doing what we have done actually preserves C# semantics. By using LINQ, you have decided to let someone else construct your queries for you. Because of this, you shouldn't care how they get translated, just that are translated properly. So, if you hadn't seen the generated query and simply looked at the shell transcript posted by Robert above, which results are more semantically equivalent. I would argue that I should see exactly the opposite of x => x.BoolField, which is exactly what has been provided. Do you disagree? | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Zaid Masud [ 19/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
I think your index argument is reasonable. However, as far as I know from a C# language perspective, the expression !BoolField is considered semantically equivalent to BoolField == false. It's definitely useful to have a way to say $ne: true (as shown in your example), but I would expect the equivalent C# expression to be BoolField != true as opposed to !BoolField, since !BoolField is semantically equivalent to BoolField == false. | |||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 17/Oct/12 ] | |||||||||||||||||||||||||||||||||||||||||||||||||
|
This is actually implemented the way it is on purpose. The reason is that if (d => d.BoolField) returns one set of documents, then (d => !d.BoolField) should return the exact opposite set of documents. This only works correctly when translated to { BoolField : { $ne : true }}. Here's a shell transcript showing how { BoolField : false }is not always the same thing as { BoolField : { $ne : true }}.
Also, using $ne does not preclude use of an index, as shown by:
That being said, an index on a boolean field is not very useful since it only has a cardinality of 2! However, if the field actually had a diverse set of data types (so only some of the values were booleans) then the index could still make sense. |