[CSHARP-497] chaining .Take() operators doesn Created: 15/Jun/12 Updated: 20/Jul/12 Resolved: 19/Jul/12 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.4.2 |
| Fix Version/s: | 1.6 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Igor Udovichenko | Assignee: | Craig Wilson |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Comments |
| Comment by auto [ 16/Jul/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'date': u'2012-07-16T08:39:13-07:00', u'email': u'craiggwilson@gmail.com', u'name': u'Craig Wilson'}Message: | |||||||||||||||||||||||||||||||||||||||||||
| Comment by auto [ 16/Jul/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'date': u'2012-07-16T08:27:17-07:00', u'email': u'craiggwilson@gmail.com', u'name': u'Craig Wilson'}Message: | |||||||||||||||||||||||||||||||||||||||||||
| Comment by auto [ 16/Jul/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'date': u'2012-07-16T07:16:08-07:00', u'email': u'craiggwilson@gmail.com', u'name': u'Craig Wilson'}Message: | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Anton Samarskyy [ 17/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
@Craig, | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 16/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
@Anton and @Igor: In relation to this issue, as Robert is indicating, we can certainly do more than we are now as we apparently have a bug. It is tentatively scheduled to get fixed in 1.6, which is likely some months away as we are preparing now to release 1.5. Even in that amount of time, there is a lot to do and we are limited by both our time and the abilities of the mongo query language. Finally, supporting certain things from Linq is either impossible or would be highly improper. For instance, we don't support joins because MongoDB does not support joins. This, by all definitions, means we have failed PI. We could support joins client-side, but the performance of the driver would suffer immeasurably. Therefore, we will not be supporting joins and any OData queries that generate a join will fail. I hope this is understandable and acceptable. | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Anton Samarskyy [ 16/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
What is the purpose of saying that Mongo Driver supports LINQ without persistence ignorance? Since we still need to create extra-layer decorating such a behavior. Is there any way to "say" Mongo Driver that collection.AsQueryable().Take(10).Skip(20).Take(50) should return the same as Linq-to-Object returns? Main concert highly related to WebApi+OData that becomes more and more popular these days: I understand it is hard to support really complicated chains of duplicated expressions, but all I need is Mongo driver to query WebApi+OData service request with same result as Linq-To-Objects would have, and be optimal | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
All I'm saying is that the LINQ semantics for chained sequences of Take/Skip can't always be mapped to an equivalent MongoDB query. I'm not saying we should do the wrong thing! For example, maybe we can convert .Take(20).Skip(5) into .Skip(5).Take(15)? But how many combinations can we manipulate this way and will we get it wrong sometimes? That's why I said: "If it gets too complicated I would vote for 1) instead, and throw an exception." At the very least we should throw an exception if we can't support the Skip and Take combinations encountered. We know that Skip followed by Take is directly supported by the MongoDB query language, so the question is how to handle all other combinations | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Igor Udovichenko [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Interesting. Seems weird to me - fails persistence ignorance here | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
In LINQ to objects, yes. But in the MongoDB query language the Skip is always done before the Take. So for example, we get the same response from the server whether we specify the limit or the skip first:
| |||||||||||||||||||||||||||||||||||||||||||
| Comment by Igor Udovichenko [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Regarding skip and take | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Regarding Skip and Take, we probably should require that Skip be before Take, since that's the only combination of Skip and Take the server supports. I don't really have a problem limiting Skip and Take to combinations that are valid for MongoDB queries. Everything in LINQ to MongoDB is basically limited by what the MongoDB query language supports. | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
I think LINQ to objects doesn't combine the Takes in any way. As data flows through the pipeline all the Takes are executed, with the net result that the smallest Take ends up determining the final result. For example, the output for:
is
| |||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Throwing an exception would cause our reporter to not work either. However, what we do is already different than what linq to objects does. For instance, we allow Take(20).Skip(20) and send those over the wire as skip() and limit(), not taking into account the order. So, while we would return items 20-40 from mongodb, linq to objects would return 0. So taking just the lowest number is probably just fine since we are already a little off. Perhaps we should enforce that skip cannot come after a take? | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Igor Udovichenko [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Yes, I prefer number 2, not only because linq2objects resolved chained Takes this way, but because collection.Where.Where has similar behavior. | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
If it gets too complicated I would vote for 1) instead, and throw an exception. | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
I like that. We need to be careful about stuff in the middle... Take(40).Skip(20).Take(20) is not the same as Take(20).Skip(20). | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Robert Stam [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Wouldn't chaining multiple Takes in LINQ to objects result in the smallest If so, perhaps the right fix is: 4) Use the smallest of the multiple Takes | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Also sounds like a bug in whatever client is generating your OData query... Regardless, we need to fix this on our side as well. These are the options, which one makes the most sense. 1) Throw an exception if multiple takes are specified I assume you want us to use #2, which is what linq to objects does, not because it checks for this, but rather because it just happens that way. | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Igor Udovichenko [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Sure, it was posted by an accident. collection.AsQueryable().Take(1).Take(50) - first Take is ignored, and resulting collection contains 20 elements, instead of one | |||||||||||||||||||||||||||||||||||||||||||
| Comment by Craig Wilson [ 15/Jun/12 ] | |||||||||||||||||||||||||||||||||||||||||||
|
Igor, could you complete your statement title and add a description? I'm not sure what you are reporting... |