CSHARP-4443: Add comprehensive dictionary LINQ support for all 3 representations #1804
Conversation
There was a problem hiding this comment.
Note: I've only reviewed the tests, where I have a lot of suggestions about possible MQL for things you didn't implement or "better" MQL in some cases for things you did implement.
Please take all suggestions with a grain of salt. You may have reasons for thinking that what I think is "better" MQL might not actually be better.
I am postponing reviewing the implementation because if you accept many of the MQL suggestions you will need to make many changes to the implementation.
I will review the implementation after you have accepted/rejected the MQL suggestions.
|
|
||
| namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira | ||
| { | ||
| public class CSharp2509Tests : LinqIntegrationTest<CSharp2509Tests.ClassFixture> |
There was a problem hiding this comment.
I was surprised that these tests were deleted, but Adelin says they duplicate new tests he added in CSharp4443Tests.cs.
We don't usually remove tests... why bother?
There was a problem hiding this comment.
Note to other reviewers: we have to manually verify that we haven't lost any test coverage as a result of deleting these tests.
| [Fact] | ||
| public void Projecting_dictionary_keys_with_arrayOfArrays_should_throw() | ||
| { | ||
| var exception = Record.Exception(() => |
There was a problem hiding this comment.
Possible MQL for this:
{ $map : { input : '$Dictionary', as : 'kvp', in : { $arrayElemAt : ['$$i', 0] } }
There was a problem hiding this comment.
For any comments below where I just show some MQL consider it as a suggestion for possible/better MQL.
| [Fact] | ||
| public void Projecting_dictionary_keys_with_arrayOfDocs_should_throw() | ||
| { | ||
| var exception = Record.Exception(() => |
| [Fact] | ||
| public void Projecting_dictionary_keys_with_document_should_throw() | ||
| { | ||
| var exception = Record.Exception(() => |
There was a problem hiding this comment.
{ $map : { input : { $objectToArray : '$Dictionary' }, as : 'kvp', in : '$$kvp.k' } }
| [Fact] | ||
| public void Projecting_dictionary_values_with_arrayOfArrays_should_throw() | ||
| { | ||
| var exception = Record.Exception(() => |
There was a problem hiding this comment.
{ $map : { input : '$Dictionary', as : 'kvp', in : { $arrayElemAt : ['$$kvp', 1] } } }
| .Where(x => x.Dictionary.ContainsValue(25)); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $match : { $expr : { $reduce : { input : { $objectToArray : '$Dictionary' }, initialValue : false, in : { $cond : { if : '$$value', then : true, else : { $eq : ['$$this.v', 25] } } } } } } }"); |
There was a problem hiding this comment.
{ $match : { $expr : { $in : [25, { $map : { input : { $objectToArray : '$Dictionary' }, as : 'kvp', in : '$$kvp.v' } }] } } }
| .Where(x => x.Dictionary.Values.Contains(42)); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $match : { $expr : { $reduce : { input : { $objectToArray : '$Dictionary' }, initialValue : false, in : { $cond : { if : '$$value', then : true, else : { $eq : ['$$this.v', 42] } } } } } } }"); |
There was a problem hiding this comment.
{ $match : { $expr : { $in : [42, { $map : { input : { $objectToArray : '$Dictionary' }, as : 'kvp', in : '$$kvp.v' } }] } } }
| .Where(x => x.DictionaryInterface["life"] == 42); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $match : { $expr : { $eq : [{ $let : { vars : { this : { $arrayToObject : '$DictionaryInterface' } }, in : '$$this.life' } }, 42] } } }"); |
There was a problem hiding this comment.
{ $match : { DictionaryInterface : { $elemMatch : { '0' : 'life', '1' : 42 } } } }
| .Where(x => x.DictionaryInterface["life"] == 42); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $match : { $expr : { $eq : [{ $let : { vars : { this : { $arrayElemAt : [{ $filter : { input : '$DictionaryInterface', as : 'kvp', cond : { $eq : ['$$kvp.k', 'life'] } } }, 0] } }, in : '$$this.v' } }, 42] } } }"); |
There was a problem hiding this comment.
{ $match : { DictionaryInterface : { $elemMatch : { k : 'life', v : 42 } } } }
| .Where(x => x.DictionaryInterface.Values.Contains(42)); | ||
|
|
||
| var stages = Translate(collection, queryable); | ||
| AssertStages(stages, "{ $match : { $expr : { $reduce : { input : { $objectToArray : '$DictionaryInterface' }, initialValue : false, in : { $cond : { if : '$$value', then : true, else : { $eq : ['$$this.v', 42] } } } } } } }"); |
There was a problem hiding this comment.
{ $match : { $expr : { $in : [42, { $map : { input : { $objectToArray : '$DictionaryInterface' }, as : 'kvp', in : '$$kvp.v' } }] } }}
2 participants
No description provided.