[CSHARP-4332] Casting/serialization issue after switching to LINQ3 Created: 26/Sep/22 Updated: 27/Oct/23 Resolved: 29/Sep/22 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | LINQ3 |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Unknown |
| Reporter: | James Kovacs | Assignee: | Robert Stam |
| Resolution: | Works as Designed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
In my codebase I have a custom type defined together with the custom serializer for that type. After switching to linq v3 several types of queries are exploding with the following error:
Is there a way to configure the driver so the query rewrite won’t be necessary? Below full repro (driver 2.17.1):
Originally from this MongoDB Community Forums post filed by Marek Olszewski. |
| Comments |
| Comment by Robert Stam [ 29/Sep/22 ] |
|
See the new CSharp4332Tests.cs file that shows how to modify the last 4 tests so that they work with LINQ. The basic principle is that all conversions between Guid and InvoiceId have to happen client side because in general we can't translate implicit conversions to server-side MQL. I would probably recommend you not have an implicit conversion from InvoiceId to Guid. Either convert explicitly, or remove the InvoiceId type and use Guid directly. |
| Comment by Githook User [ 29/Sep/22 ] |
|
Author: {'name': 'rstam', 'email': 'robert@robertstam.org', 'username': 'rstam'}Message: |
| Comment by Robert Stam [ 27/Sep/22 ] |
|
In this message I explain why I don't think c => c.InvoiceId == guidNullable can be correctly translated. This expression is comparing apples to oranges (or rather InvoiceIds to Guids). It only compiles because an implicit conversion is defined from InvoiceId => Guid. After implicit conversions are applied this expression is actually: c => (Convert(Convert(c.InvoiceId, Guid), Nullable`1) == CSharp4332Tests.guidNullable) The problematic conversion is the inner one, where `c.InvoiceId` is converted to a `Guid` using the implicit conversion defined in the `InvoiceId` struct. To translate this to MQL the equivalent conversion would have to be done server-side. But what the implicit conversion actually does is totally unknown to the LINQ translator. It could be doing anything, so we have no idea what server-side MQL might possibly do the same thing server-side. Why does this work in LINQ2? By a combination of coincidences. The first coincidence is that LINQ2 (for other reasons unrelated to this) actually removes all `Convert` nodes from the expression before translating it. This is wrong in the general case because we have no way of knowing what the `Convert` actually does, so that is actually a bug in LINQ2. The second coincidence is that your implicit conversion is essentially an identity transformation. Both before and after the conversion from `InvoiceId` to `Guid` the serialized representation is the same `Guid` in `string` form. It is only because the two representations are coincidentally the same that ignoring the `Convert` nodes in LINQ2 just happened to result in a query that returned the expected documents. In the general case the implied conversion might have done some actual work. To imagine how this might make ignoring the `Convert` nodes incorrect, imagine that converting from an `InvoiceId` to a `Guid` involved reversing the bytes in the Guid for some reason (or any other transformation that resulted in a different serialized form). I'm going to close this ticket as works as designed because I don't think the 4 cases that are throwing an exception can do anything different, Throwing an exception appears to be the correct thing to do.
|
| Comment by Robert Stam [ 26/Sep/22 ] |
|
See my attempt to repro on the following branch: https://github.com/mongodb/mongo-csharp-driver/pull/884 I can confirm that the last 4 test cases throw exceptions when using LINQ3, but I will explain later why they can't be correctly implemented in the general case, and why they happened to work in LINQ2 by coincidence only. But first, I had to make some changes in order to incorporate your example into our xUnit test suite. I did not use `BsonSerializer.RegisterSerializer` because that modifies the global serializer registry which would cause other tests to fail. Instead I configured the specific test classes in this example to serialize as desired. Instead of setting `guid` to a random `Guid` I set it to a specific constant so that the test is repeatable. I used a `[Theory]` test method named `Where_expression_should_work` that is called 16 times with the `i` parameter varying from `0` to `15`. The `i` parameter is then used to extract a test case from the `__testCases` static array. I renamed your `MyGuidSerializer` class to `InvoiceIdSerializer` because I found the name confusing (it does not serialize `Guid`s as the name implies, but rather `InvoiceId`s). As a general rule a serializer for class `Xyz` should be called `XyzSerializer`. I left out the `MyGuidBsonTypeMapper` class because I don't think it has any role to play. I modified the last 4 test cases so that the type conversions happen client side (not server side) and therefore can be correctly translated. In the following comment I will describe why I think the last 4 cases can't be translated correctly (and therefore throwing an `ExpressionNotSupportedException` is the correct thing to do). |