[CSHARP-3886] Improve LINQ3 KnownSerializers to better handle enums Created: 01/Oct/21 Updated: 05/May/23 |
|
| Status: | Backlog |
| Project: | C# Driver |
| Component/s: | LINQ3 |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Unknown |
| Reporter: | James Kovacs | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
| Description |
|
Because of the way that .NET/C# treat enums, known serializers requires special code for handling them. Consider the following expression: x => x.Es == E.A where E is an enum and Es is an enum property configured to use the string enum representation. This expression tree appears in code as: x => Convert(x.Es, Int32) == 0 The enum value E.A is translated into 0 (zero) without any indication that it was ever an enum. So our serializer finder code has to say “oh, here’s an integer, I wonder if it might be an enum… let’s start walking up the tree to see if there is an enum serializer configured somewhere above us…” Most of the time it works because the immediate node above has the correct enum serializer configured. But it is possible to come up with cases where we serialize an actual integer as an enum when it should be serialized as a numeric type. For example: x => x.P == (x.Es == E.A ? 42 : 144) where x.P is an integer property and x.Es and E.A are as defined above. When we ask KnownSerializersRegistry for serializers for 42 and 144, it will return the string enum serializer because it was pushed up from the x.Es node rather than the Int32Serializer from the x.P node. Note that this can only happen when enums appear in the expression tree. In the most common cases when comparing a document field to an enum, the correct serializer is chosen. The default enum serializer uses an int32 representation - which although it is the incorrect serializer for a true int32 from a conceptual standpoint - the resulting representation will be identical; the BSON document will contain an int32. Thus our initial analysis of the problem is that although this is a shortcoming of the current known serializers implementation, it is unlikely to affect most users. |
| Comments |
| Comment by James Kovacs [ 04/May/23 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hi, robin.kaulfuss@qualitybytes.de, Thank you for reaching out and providing code examples of a similar problem that you encountered. When translating an expression in LINQ, the LINQ provider receives an expression tree containing CLR constructs, not C# ones. Thus in the expression tree, the C# compiler has already translated any enum constants to their corresponding underlying numeric value. While any enum property or return value still has the enum type, any constants (for comparisons, assignments, etc.) have been replaced with their equivalent numeric representation. When we encounter numeric values in an expression tree, we have to figure out which might have originally been enums so that we can render the correct representation in the MQL. There are cases such as you encountered where we make the wrong determination. I'm glad that you were able to find a workaround. Your test cases are helpful as we investigate how best to fix edge cases such as this. Please continue to follow this ticket for updates. Sincerely, James | |||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Robin Kaulfuß [ 04/May/23 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
Stumbled upon this, having EnumAsString convention.
only workaround was to skip the limit and move the Take(1) to a second Project stage.
I`m pretty sure it originates from ConstantExpressionToAggregationExpressionTranslator
and KnownSerializersNode.GetPossibleSerializersAtThisLevel
but I don`t know why the underlying type is compared here. Aren`t enums supposed to be expressed as enums instead of int or string etc.? | |||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by James Kovacs [ 01/Oct/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
Repro:
|