CSHARP-5463: Investigate whether calls to EnsureQueryableMethodHasNestedAsQueryableSource are actually needed. #1669
Conversation
| @@ -27,7 +27,7 @@ public static void EnsureQueryableMethodHasNestedAsQueryableSource(MethodCallExp | |||
| if (expression.Method.DeclaringType == typeof(Queryable) && | |||
| sourceTranslation.Serializer is not INestedAsQueryableSerializer) | |||
| { | |||
| throw new ExpressionNotSupportedException(expression, because: "source serializer is not a NestedAsQueryableSerializer"); | |||
| throw new ExpressionNotSupportedException(expression, because: "source argument is an unsupported IQueryable type"); | |||
There was a problem hiding this comment.
Maybe the message should be even more descriptive?
For example, it could include the type of the unsupported IQueryable...
There was a problem hiding this comment.
I don't think we can really improve the error message:
- The type of IQueryable is not known unless the value is a constant
- The name of the type doesn't mean anything to the user anyway (e.g.
EnumerableQuery)
| var enumerable = new int[] { 1, 2, 3 }; | ||
|
|
||
| var queryable = collection.AsQueryable() | ||
| .Select(x => enumerable.AsQueryable().Contains(1)); |
There was a problem hiding this comment.
This is the nested AsQueryable we've been supporting all along.
| var nestedQueryable = new int[] { 1, 2, 3 }.AsQueryable(); | ||
|
|
||
| var queryable = collection.AsQueryable() | ||
| .Select(x => nestedQueryable.Contains(1)); // PartialEvaluator turns this into Select(x => true) |
There was a problem hiding this comment.
This one doesn't trigger an error because the entire nested queryable expression is evaluated by the partial evaluator.
| var nestedQueryable = new int[] { 1, 2, 3 }.AsQueryable(); | ||
|
|
||
| var queryable = collection.AsQueryable() | ||
| .Select(x => nestedQueryable.Contains(x.Id)); |
There was a problem hiding this comment.
This one triggers an error because the translator doesn't know how to translate nestedAsQueryable.
| .Select(x => enumerable.AsQueryable().Contains(1)); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $project : { _v : { $in : [1, [1, 2, 3]] }, _id : 0 } }"); |
There was a problem hiding this comment.
I would say it's danger to support such translation. It could render the entire collection into the wire. Is such translation works with LinqToSql or something like that? It could lead to loading the entire table into the memory and then rendering it into the MQL. We might want to forbid this.
There was a problem hiding this comment.
This is a NESTED AsQueryable.
It's the same thing as if they left out the AsQueryable.
…tedAsQueryableSource are actually needed.
We should probably have a Zoom meeting to discuss this one.