[CSHARP-1186] LINQ skip implementation is not consistent with native linq to objects Created: 02/Mar/15 Updated: 03/Dec/20 Resolved: 03/Dec/20 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | Linq |
| Affects Version/s: | 1.9.2 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Trivial - P5 |
| Reporter: | Scott Guymer | Assignee: | Unassigned |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
When using the skip method with a negative number the mongo driver raises the following exception ArgumentException: Skip cannot be negative. However this is not consistent with the linq to objects implementation and therefore makes for the possibility of runtime errors slipping through unit tests when linq is using the mongodb driver. |
| Comments |
| Comment by Rahman Mukras [ 09/Mar/15 ] | ||||||||||||||||||||
|
Thanks for this Craig, | ||||||||||||||||||||
| Comment by Craig Wilson [ 09/Mar/15 ] | ||||||||||||||||||||
|
Perhaps you're right. The LINQ provider may end up calling here. If that's the case, we can't change this behavior as it's the correct behavior in one instance and the "wrong" behavior in another. | ||||||||||||||||||||
| Comment by Rahman Mukras [ 09/Mar/15 ] | ||||||||||||||||||||
|
Hi Craig, I am at odds at where else to look, and any pointers would be appreciated... | ||||||||||||||||||||
| Comment by Craig Wilson [ 09/Mar/15 ] | ||||||||||||||||||||
|
Hi Rahman, This code is not the problem. Since MongoDB doesn't support negative skips, we really should be throwing in this case. This ticket is about the LINQ implementation that does throw where LINQ to objects does not throw. That code will be in a different place. Craig | ||||||||||||||||||||
| Comment by Rahman Mukras [ 09/Mar/15 ] | ||||||||||||||||||||
|
I wanted to pick this nice and small issue just to get started, but could not assign it to myself and so I could not assume that no one has started work on it. Is anyone working on this? In any case, here are the two diffs of the test and code that I plan to submit.
| ||||||||||||||||||||
| Comment by Craig Wilson [ 02/Mar/15 ] | ||||||||||||||||||||
|
Our process is pretty loose. Unfortunately, we have a number of PRs sitting there and haven't managed that backlog very well. We are deep into trying to get 2.0.0 out and this is going to slip through the cracks. If you'd like to submit a PR, submit against 1.x and put a link in this ticket and we'll get to it once 2.0.0 is released. Thanks. | ||||||||||||||||||||
| Comment by Scott Guymer [ 02/Mar/15 ] | ||||||||||||||||||||
|
Im more than happy to make the fix and submit the PR myself. Although i admit i havent read your process for allowing people to contribute yet! | ||||||||||||||||||||
| Comment by Scott Guymer [ 02/Mar/15 ] | ||||||||||||||||||||
|
Thanks for this Craig Totally understand what you are saying about unit tests but testing against Mongo would be part of our integrations tests rather than unit testing the logic in the code. The issue was more that is was less the fact that it wasnt supported by the driver but more that it was raising an unexpected exception at run time. | ||||||||||||||||||||
| Comment by Craig Wilson [ 02/Mar/15 ] | ||||||||||||||||||||
|
You're right. It looks like Linq to Objects treats negative skips as 0.
So, I'll re-open this and we'll discuss changing our behavior. I'd like to caution you that writing unit tests against LINQ to Objects and then running your code against MongoDB (or any other queryable provider) will probably end up badly because no provider will support the full breadth of the possible operators and methods. | ||||||||||||||||||||
| Comment by Scott Guymer [ 02/Mar/15 ] | ||||||||||||||||||||
|
Dont think its a case of supporting negative skips - im not sure in what circumstance it would be needed Its more of a case that linq to objects seems to absorb the problem (maybe transparently setting to 0) where as the mongo implementation throws an exception that you cant test for in a unit test with going full on integration test. I havent investigated the .net core code to see what it does but it doesnt throw an exception. | ||||||||||||||||||||
| Comment by Craig Wilson [ 02/Mar/15 ] | ||||||||||||||||||||
|
Hi Scott, Thanks for the report. While we do strive to be consistent with Linq to Objects as much as possible, there are a great number of things that simply aren't supported at all. LINQ is a great big leaky abstraction since not all data sources provide the same features. This is one of those. MongoDB simply doesn't support negative skips. I'm closing this as won't fix. Feel free to respond if you feel this is a mistake and how you think we could support this feature. Thanks, |