-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-4443: Add comprehensive dictionary LINQ support for all 3 representations #1804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bc008db
462dc1c
205eb56
4c2d0bb
f1d98e4
94484c9
a416316
fdb6fa7
0cb78dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,6 @@ | |
| */ | ||
|
|
||
| using System; | ||
| using System.Collections; | ||
| using System.Collections.Generic; | ||
| using System.Linq.Expressions; | ||
| using System.Reflection; | ||
|
|
@@ -71,11 +70,16 @@ public static TranslatedExpression Translate(TranslationContext context, MemberE | |
|
|
||
| if (!DocumentSerializerHelper.AreMembersRepresentedAsFields(containerTranslation.Serializer, out _)) | ||
| { | ||
| if (member is PropertyInfo propertyInfo && propertyInfo.Name == "Length") | ||
| if (member is PropertyInfo propertyInfo && propertyInfo.Name == "Length") | ||
| { | ||
| return LengthPropertyToAggregationExpressionTranslator.Translate(context, expression); | ||
| } | ||
|
|
||
| if (TryTranslateKeyValuePairProperty(expression, containerTranslation, member, out var translatedKeyValuePairProperty)) | ||
| { | ||
| return translatedKeyValuePairProperty; | ||
| } | ||
|
|
||
| if (TryTranslateCollectionCountProperty(expression, containerTranslation, member, out var translatedCount)) | ||
| { | ||
| return translatedCount; | ||
|
|
@@ -126,11 +130,20 @@ private static bool TryTranslateCollectionCountProperty(MemberExpression express | |
| { | ||
| if (EnumerableProperty.IsCountProperty(expression)) | ||
| { | ||
| SerializationHelper.EnsureRepresentationIsArray(expression, container.Serializer); | ||
| AstExpression ast; | ||
|
|
||
| var ast = AstExpression.Size(container.Ast); | ||
| var serializer = Int32Serializer.Instance; | ||
| if (container.Serializer is IBsonDictionarySerializer dictionarySerializer && | ||
| dictionarySerializer.DictionaryRepresentation == DictionaryRepresentation.Document) | ||
| { | ||
| ast = AstExpression.Size(AstExpression.ObjectToArray(container.Ast)); | ||
| } | ||
| else | ||
| { | ||
| SerializationHelper.EnsureRepresentationIsArray(expression, container.Serializer); | ||
| ast = AstExpression.Size(container.Ast); | ||
| } | ||
|
|
||
| var serializer = Int32Serializer.Instance; | ||
| result = new TranslatedExpression(expression, ast, serializer); | ||
| return true; | ||
| } | ||
|
|
@@ -213,6 +226,16 @@ private static bool TryTranslateDictionaryProperty(TranslationContext context, M | |
|
|
||
| switch (propertyInfo.Name) | ||
| { | ||
| case "Count": | ||
| var countAst = dictionaryRepresentation switch | ||
| { | ||
| DictionaryRepresentation.ArrayOfDocuments or DictionaryRepresentation.ArrayOfArrays => AstExpression.Size(containerAst), | ||
| _ => throw new ExpressionNotSupportedException(expression, $"Unexpected dictionary representation: {dictionaryRepresentation}") | ||
| }; | ||
| var countSerializer = Int32Serializer.Instance; | ||
| translatedDictionaryProperty = new TranslatedExpression(expression, countAst, countSerializer); | ||
| return true; | ||
|
|
||
| case "Keys": | ||
| var keysAst = dictionaryRepresentation switch | ||
| { | ||
|
|
@@ -261,5 +284,36 @@ private static bool TryTranslateDictionaryProperty(TranslationContext context, M | |
| translatedDictionaryProperty = null; | ||
| return false; | ||
| } | ||
|
|
||
| private static bool TryTranslateKeyValuePairProperty(MemberExpression expression, TranslatedExpression container, MemberInfo memberInfo, out TranslatedExpression result) | ||
| { | ||
| result = null; | ||
|
|
||
| if (container.Expression.Type.IsGenericType && | ||
| container.Expression.Type.GetGenericTypeDefinition() == typeof(KeyValuePair<,>) && | ||
| container.Serializer is IKeyValuePairSerializerV2 { Representation: BsonType.Array } kvpSerializer) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we ignoring |
||
| { | ||
| AstExpression ast; | ||
| IBsonSerializer serializer; | ||
|
|
||
| switch (memberInfo.Name) | ||
| { | ||
| case "Key": | ||
| ast = AstExpression.ArrayElemAt(container.Ast, 0); | ||
| serializer = kvpSerializer.KeySerializer; | ||
| break; | ||
| case "Value": | ||
| ast = AstExpression.ArrayElemAt(container.Ast, 1); | ||
| serializer = kvpSerializer.ValueSerializer; | ||
| break; | ||
| default: | ||
| throw new ExpressionNotSupportedException(expression); | ||
| } | ||
| result = new TranslatedExpression(expression, ast, serializer); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| using MongoDB.Driver.Linq.Linq3Implementation.Ast.Expressions; | ||
| using MongoDB.Driver.Linq.Linq3Implementation.Misc; | ||
| using MongoDB.Driver.Linq.Linq3Implementation.Reflection; | ||
| using MongoDB.Driver.Linq.Linq3Implementation.Serializers; | ||
|
|
||
| namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators.MethodTranslators | ||
| { | ||
|
|
@@ -123,7 +124,21 @@ public static TranslatedExpression Translate(TranslationContext context, MethodC | |
| } | ||
| else | ||
| { | ||
| ast = AstExpression.Avg(sourceTranslation.Ast); | ||
| var sourceItemSerializer = ArraySerializerHelper.GetItemSerializer(sourceTranslation.Serializer); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is similar code in the translators for Max, Min and Sum. Is there any way to share the code? No problem if not. |
||
| if (sourceItemSerializer is IWrappedValueSerializer wrappedValueSerializer) | ||
| { | ||
| var itemVar = AstExpression.Var("item"); | ||
| var unwrappedItemAst = AstExpression.GetField(itemVar, wrappedValueSerializer.FieldName); | ||
| ast = AstExpression.Avg( | ||
| AstExpression.Map( | ||
| input: sourceTranslation.Ast, | ||
| @as: itemVar, | ||
| @in: unwrappedItemAst)); | ||
| } | ||
| else | ||
| { | ||
| ast = AstExpression.Avg(sourceTranslation.Ast); | ||
| } | ||
| } | ||
| IBsonSerializer serializer = expression.Type switch | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| * limitations under the License. | ||
| */ | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Linq.Expressions; | ||
| using MongoDB.Bson.Serialization.Serializers; | ||
| using MongoDB.Driver.Linq.Linq3Implementation.Ast.Expressions; | ||
|
|
@@ -33,6 +34,11 @@ public static TranslatedExpression Translate(TranslationContext context, MethodC | |
|
|
||
| if (IsEnumerableContainsMethod(expression, out var sourceExpression, out var valueExpression)) | ||
| { | ||
| if (TryTranslateDictionaryKeysOrValuesContains(context, expression, sourceExpression, valueExpression, out var dictionaryTranslation)) | ||
| { | ||
| return dictionaryTranslation; | ||
| } | ||
|
|
||
| return TranslateEnumerableContains(context, expression, sourceExpression, valueExpression); | ||
| } | ||
|
|
||
|
|
@@ -83,5 +89,48 @@ private static TranslatedExpression TranslateEnumerableContains(TranslationConte | |
|
|
||
| return new TranslatedExpression(expression, ast, BooleanSerializer.Instance); | ||
| } | ||
|
|
||
| private static bool TryTranslateDictionaryKeysOrValuesContains( | ||
| TranslationContext context, | ||
| Expression expression, | ||
| Expression sourceExpression, | ||
| Expression valueExpression, | ||
| out TranslatedExpression translation) | ||
| { | ||
| translation = null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above about assigning null to out parameter at top of method. |
||
|
|
||
| if (sourceExpression is not MemberExpression memberExpression) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| var memberName = memberExpression.Member.Name; | ||
| var declaringType = memberExpression.Member.DeclaringType; | ||
|
|
||
| if (!declaringType.IsGenericType || | ||
| (declaringType.GetGenericTypeDefinition() != typeof(Dictionary<,>) && | ||
| declaringType.GetGenericTypeDefinition() != typeof(IDictionary<,>))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 110-112 might be simpler like this: also: test with IReadOnlyDictionary |
||
| { | ||
| return false; | ||
| } | ||
|
|
||
| switch (memberName) | ||
| { | ||
| case "Keys": | ||
| { | ||
| var dictionaryExpression = memberExpression.Expression; | ||
| translation = ContainsKeyMethodToAggregationExpressionTranslator.TranslateContainsKey(context, expression, dictionaryExpression, valueExpression); | ||
| return true; | ||
| } | ||
| case "Values": | ||
| { | ||
| var dictionaryExpression = memberExpression.Expression; | ||
| translation = ContainsValueMethodToAggregationExpressionTranslator.TranslateContainsValue(context, expression, dictionaryExpression, valueExpression); | ||
| return true; | ||
| } | ||
| default: | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like assigning null to out variables at the beginning of the method.
I realize it makes it more convenient to return false, but it also disables the compiler warning you get if you try to return true without setting an out parameter.
Your call.