CSHARP-5779: Support Dictionary Keys and Values properties #1807

rstam · 2025-11-07T23:51:32Z

No description provided.

rstam · 2025-11-07T23:51:55Z

src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Expressions/AstExpression.cs

            return new AstComputedDocumentExpression(fields);
        }

+        public static AstExpression ComputedDocument(IEnumerable<(string Name, AstExpression Value)> fields)


A more convenient overload of ComputedDocument.

rstam · 2025-11-07T23:52:48Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryKeyCollectionSerializer.cs

+    }
+}
+
+internal class DictionaryKeyCollectionSerializer<TKey, TValue> : EnumerableSerializerBase<Dictionary<TKey, TValue>.KeyCollection>


We need a serializer for Dictionary<TKey, TValue>.KeyCollection to be able to work with this type in LINQ queries or to deserialize them client-side.

rstam · 2025-11-07T23:53:03Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryValueCollectionSerializer.cs

+    }
+}
+
+internal class DictionaryValueCollectionSerializer<TKey, TValue> : SerializerBase<Dictionary<TKey, TValue>.ValueCollection>, IBsonArraySerializer


We need a serializer for Dictionary<TKey, TValue>.ValueCollection to be able to work with this type in LINQ queries or to deserialize them client-side.

rstam · 2025-11-07T23:53:33Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionarySerializer.cs

+    }
+}
+
+internal class DictionarySerializer<TKey, TValue> : DictionarySerializerBase<Dictionary<TKey, TValue>, TKey, TValue>


This is just a more convenient way to create a serializer for Dictionary<TKey, TValue>.

rstam · 2025-11-07T23:53:57Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/ICollectionSerializer.cs

+    }
+}
+
+internal class ICollectionSerializer<TItem> : EnumerableSerializerBase<ICollection<TItem>>


This is just a more convenient way to create a serializer for ICollection<TItem>.

rstam · 2025-11-07T23:59:00Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/KeyValuePairWrappedValueSerializer.cs

+    }
+}
+
+internal class KeyValuePairWrappedValueSerializer<TKey, TValue> : SerializerBase<TValue>, IWrappedValueSerializer


This is a special serializer that deserializes a KeyValuePair but then extracts only the Value portion of it as the result.

It also represents values that are still coupled with the corresponding key as needed to work with a Dictionary<TKey, TValue>.ValueCollection.

rstam · 2025-11-07T23:59:32Z

.../ExpressionToAggregationExpressionTranslators/ExpressionToAggregationExpressionTranslator.cs

    {
        // public static methods
        public static TranslatedExpression Translate(TranslationContext context, Expression expression)
+        {


Whenever we want to use a value we go ahead and unwrap it if it is wrapped.

rstam · 2025-11-07T23:59:52Z

.../ExpressionToAggregationExpressionTranslators/ExpressionToAggregationExpressionTranslator.cs


                var ast  = AstExpression.ObjectToArray(aggregateExpression.Ast);
-                var ienumerableSerializer = ArraySerializerHelper.CreateSerializer(keyValuePairSerializer);
+                var arrayOfDocumentsDictionarySerializer = DictionarySerializer.Create(DictionaryRepresentation.ArrayOfDocuments, keySerializer, valueSerializer);


We were creating the "wrong" serializer before.

rstam · 2025-11-08T00:01:14Z

...ssionToAggregationExpressionTranslators/MemberExpressionToAggregationExpressionTranslator.cs

+                            case "Values":
+                                if (declaringTypeDefinition == typeof(Dictionary<,>))
+                                {
+                                    var kvpPairsAst = dictionaryRepresentation switch


The tricky part about supporting Values is that we have to keep the keys that go with them, at least until some later time where the keys might no longer be needed.

rstam · 2025-11-08T00:01:55Z

...ssionToAggregationExpressionTranslators/MemberExpressionToAggregationExpressionTranslator.cs

+                                }
+                                else if (declaringTypeDefinition == typeof(IDictionary<,>))
+                                {
+                                    var valuesAst = dictionaryRepresentation switch


Supporting Values for IDictionary<TKey, TValue> is MUCH more straightforward because the type is ICollection<TValue>.

rstam · 2025-11-08T16:08:23Z

...ssionToAggregationExpressionTranslators/MemberExpressionToAggregationExpressionTranslator.cs

+                            case "Keys":
+                                var keysAst = dictionaryRepresentation switch
+                                {
+                                    DictionaryRepresentation.ArrayOfDocuments => AstExpression.Map(containerAst, kvpVar, AstExpression.GetField(kvpVar, "k")),


By the time we get here we will never encounter DictionaryRepresentation.Document, because the call to TranslateEnumerable on line 204 above will have already used $objectToArray to transform the representation from Document to ArrayOfDocuments.

Copilot

Pull Request Overview

This pull request implements support for Dictionary Keys and Values properties in MongoDB's LINQ provider, enabling these properties to be used in queries for different dictionary representations (ArrayOfArrays, ArrayOfDocuments, and Document).

Key Changes:

Added translation logic for Dictionary and IDictionary Keys and Values properties
Introduced specialized serializers to handle dictionary key and value collections during deserialization
Refactored expression translation to separate unwrapping logic from the main translation flow

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
CSharp5779Tests.cs	Comprehensive test coverage for Dictionary/IDictionary Keys/Values operations across different serialization formats
MemberExpressionToAggregationExpressionTranslator.cs	Core translation logic for Dictionary Keys/Values properties with representation-specific AST generation
ExpressionToAggregationExpressionTranslator.cs	Refactored to extract unwrapping logic and fixed serializer usage for objectToArray operations
KeyValuePairWrappedValueSerializer.cs	New serializer for extracting values from KeyValuePair documents
ICollectionSerializer.cs	Generic serializer for ICollection deserialization
DictionaryValueCollectionSerializer.cs	Specialized serializer for Dictionary.ValueCollection
DictionarySerializer.cs	Factory for creating dictionary serializers with specific representations
DictionaryKeyCollectionSerializer.cs	Specialized serializer for Dictionary.KeyCollection
AstExpression.cs	Added convenience overload for creating computed documents from tuples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T16:37:36Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5779Tests.cs

+            .Select(x => x.IDictionaryAsArrayOfArrays.Values.First(v => v == 2));
+
+        var stages = Translate(collection, queryable);
+        AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : { $map : { input : '$IDictionaryAsArrayOfArrays', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } } , as : 'v', cond : { $eq : ['$$v', 2] } } }, 0] }, _id : 0 } }");


[nitpick] Extraneous space before the comma after the closing brace. The space before , as is inconsistent with other test assertions in this file.

Suggested change

AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : { $map : { input : '$IDictionaryAsArrayOfArrays', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } } , as : 'v', cond : { $eq : ['$$v', 2] } } }, 0] }, _id : 0 } }");

AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : { $map : { input : '$IDictionaryAsArrayOfArrays', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } }, as : 'v', cond : { $eq : ['$$v', 2] } } }, 0] }, _id : 0 } }");

Copilot · 2025-11-10T16:37:36Z

...ssionToAggregationExpressionTranslators/MemberExpressionToAggregationExpressionTranslator.cs

+                                {
+                                    var valuesAst = dictionaryRepresentation switch
+                                    {
+                                        DictionaryRepresentation.ArrayOfArrays => AstExpression.Map(containerAst, kvpVar, AstComputedArrayExpression.ArrayElemAt(kvpVar, 1)),


Inconsistent use of AstComputedArrayExpression.ArrayElemAt instead of AstExpression.ArrayElemAt. Line 220 uses AstExpression.ArrayElemAt for a similar operation, suggesting this should also use AstExpression.ArrayElemAt for consistency.

Suggested change

DictionaryRepresentation.ArrayOfArrays => AstExpression.Map(containerAst, kvpVar, AstComputedArrayExpression.ArrayElemAt(kvpVar, 1)),

DictionaryRepresentation.ArrayOfArrays => AstExpression.Map(containerAst, kvpVar, AstExpression.ArrayElemAt(kvpVar, 1)),

I agree with Copilot. Don't know how this happened... probably some weird Rider auto-completion that I didn't catch.

sanych-sun · 2025-11-10T17:23:31Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryValueCollectionSerializer.cs

+        return (IBsonSerializer)Activator.CreateInstance(serializerType, [keySerializer, valueSerializer]);
+    }
+}
+


Why DictionaryValueCollectionSerializer does not inherited from EnumerableSerializerBase as DictionaryKeyCollectionSerializer does?

Maybe it could, but I think it would probably be more complicated. See the implementation of the Deserialize method in this class.

In essence it is because for DictionaryKeyCollectionSerializer we only ever need the key values, so EnumerableSerializerBase<TKey> makes sense.

But for DictionaryValueCollectionSerializer we need the key/value pairs (not just the values), EnumerableSerializerBase<TValue> doesn't work.

sanych-sun · 2025-11-10T17:25:40Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryValueCollectionSerializer.cs

+
+    public DictionaryValueCollectionSerializer(IBsonSerializer<TKey> keySerializer,  IBsonSerializer<TValue> valueSerializer)
+    {
+        _dictionarySerializer = (IBsonSerializer<Dictionary<TKey, TValue>>)DictionarySerializer.Create(DictionaryRepresentation.ArrayOfDocuments,  keySerializer, valueSerializer);


Shouldn't we have DictionaryRepresentation as a parameter?

No, because the representation MUST be ArrayOfDocuments.

sanych-sun · 2025-11-10T17:27:54Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryValueCollectionSerializer.cs

+
+namespace MongoDB.Driver.Linq.Linq3Implementation.Serializers;
+
+internal static class DictionaryValueCollectionSerializer


I'm not sure if I really understand the difference, but it feels like Key- and Value-Collections serializers must be exactly the same, but accessing the different property of the elements. If so, can we share code in between those serializers in some way?

They have in common that the only way to create them is to create a dictionary first and then access the .Keys or the .Values property. This is a weirdness of how .NET defined them...

They are different in that for creating the keys collection all we need are the key values. We can use default for the values, because the values don't matter.

But for the values collection we need to have unique key values. The key values don't matter, but they have to be unique or we can't create the dictionary because of duplicate keys. If we COULD create unique values we could do that, but there is no way to create unique values for arbitrary types, so we fall back to using the key/value pairs from the original dictionary. This is awkward because when the user writes dictionary.Values they only want the values, but we still need the key values under the covers.

I chose two serializers because I didn't want all that nastiness for the keys collection where we don't need it.

adelinowona · 2025-11-10T22:03:54Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryKeyCollectionSerializer.cs

+    {
+        var keyType = keySerializer.ValueType;
+        var valueType = valueSerializer.ValueType;
+        var serializerType  = typeof(DictionaryKeyCollectionSerializer<,>).MakeGenericType(keyType, valueType);


there's extra space between the variable name and "=" on this line.

adelinowona · 2025-11-10T22:04:02Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/DictionaryValueCollectionSerializer.cs

+    {
+        var keyType = keySerializer.ValueType;
+        var valueType = valueSerializer.ValueType;
+        var serializerType  = typeof(DictionaryValueCollectionSerializer<,>).MakeGenericType(keyType, valueType);


there's extra space between the variable name and "=" on this line.

adelinowona · 2025-11-10T22:04:19Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/ICollectionSerializer.cs

+    public static IBsonSerializer Create(IBsonSerializer itemSerializer)
+    {
+        var itemType = itemSerializer.ValueType;
+        var serializerType  = typeof(ICollectionSerializer<>).MakeGenericType(itemType);


there's extra space between the variable name and "=" on this line.

adelinowona · 2025-11-10T22:04:29Z

src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/KeyValuePairWrappedValueSerializer.cs

+    {
+        var keyType = keySerializer.ValueType;
+        var valueType = valueSerializer.ValueType;
+        var serializerType  = typeof(KeyValuePairWrappedValueSerializer<,>).MakeGenericType(keyType, valueType);


there's extra space between the variable name and "=" on this line.

adelinowona · 2025-11-10T22:11:03Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5779Tests.cs

Should we add tests for calling Keys and Values on empty dictionaries?
Also maybe tests for calling Values on nested dictionaries -> Dictionary<string, Dictionary<string, int>>

Good idea. I will add more tests in a future commit.

adelinowona

LGTM

adelinowona

LGTM

sanych-sun

LGTM

CSHARP-5779: Support Dictionary Keys and Values properties

02f691a

rstam requested a review from adelinowona November 7, 2025 23:51

rstam added the improvement Optimizations or refactoring (no new features or fixes). label Nov 7, 2025

rstam requested a review from a team as a code owner November 7, 2025 23:51

rstam commented Nov 8, 2025

View reviewed changes

rstam requested a review from sanych-sun November 8, 2025 00:02

CSHARP-5779: Removed a small amount of unused code

6f7337c

rstam commented Nov 8, 2025

View reviewed changes

adelinowona requested a review from Copilot November 10, 2025 16:36

Copilot AI reviewed Nov 10, 2025

View reviewed changes

sanych-sun reviewed Nov 10, 2025

View reviewed changes

rstam requested a review from sanych-sun November 10, 2025 18:10

adelinowona requested changes Nov 10, 2025

View reviewed changes

rstam added 2 commits November 10, 2025 17:23

CSHARP-5779: Requested changes

7111443

CSHARP-5779: Expand tests to cover dictionaries with 0-3 values

40372f3

adelinowona previously approved these changes Nov 11, 2025

View reviewed changes

CSHARP-5779: Added tests for dictionaries with nested dictionary values

15113ca

rstam dismissed adelinowona’s stale review via 15113ca November 11, 2025 16:23

rstam requested a review from adelinowona November 11, 2025 16:25

adelinowona approved these changes Nov 11, 2025

View reviewed changes

sanych-sun approved these changes Nov 12, 2025

View reviewed changes

rstam merged commit 2c6ad30 into mongodb:main Nov 12, 2025
35 of 37 checks passed

	AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : { $map : { input : '$IDictionaryAsArrayOfArrays', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } } , as : 'v', cond : { $eq : ['$$v', 2] } } }, 0] }, _id : 0 } }");
	AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : { $map : { input : '$IDictionaryAsArrayOfArrays', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } }, as : 'v', cond : { $eq : ['$$v', 2] } } }, 0] }, _id : 0 } }");

	DictionaryRepresentation.ArrayOfArrays => AstExpression.Map(containerAst, kvpVar, AstComputedArrayExpression.ArrayElemAt(kvpVar, 1)),
	DictionaryRepresentation.ArrayOfArrays => AstExpression.Map(containerAst, kvpVar, AstExpression.ArrayElemAt(kvpVar, 1)),


		namespace MongoDB.Driver.Linq.Linq3Implementation.Serializers;

		internal static class DictionaryValueCollectionSerializer

CSHARP-5779: Support Dictionary Keys and Values properties #1807

CSHARP-5779: Support Dictionary Keys and Values properties #1807

Uh oh!

Conversation

rstam commented Nov 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanych-sun Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

sanych-sun Nov 10, 2025 •

edited

Loading