[CSHARP-4499] Support Convert calls to a base type in filter translators Created: 30/Jan/23 Updated: 28/Oct/23 Resolved: 15/Feb/23 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | LINQ3 |
| Affects Version/s: | 2.19.0 |
| Fix Version/s: | 2.19.1 |
| Type: | Bug | Priority: | Unknown |
| Reporter: | DanikRaikhlin N/A | Assignee: | Robert Stam |
| Resolution: | Fixed | Votes: | 3 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Documentation Changes: | Not Needed | ||||||||||||||||||||||||||||||||
| Documentation Changes Summary: | 1. What would you like to communicate to the user about this feature? |
||||||||||||||||||||||||||||||||
| Description |
| Comments |
| Comment by Githook User [ 24/Mar/23 ] | ||
|
Author: {'name': 'rstam', 'email': 'robert@robertstam.org', 'username': 'rstam'}Message: | ||
| Comment by Githook User [ 15/Feb/23 ] | ||
|
Author: {'name': 'rstam', 'email': 'robert@robertstam.org', 'username': 'rstam'}Message: | ||
| Comment by DanikRaikhlin N/A [ 01/Feb/23 ] | ||
|
As you mention the serialization of the TField part then it might be handy to expand on the scenario. The created FieldDefintion will eventually be used in a FilterDefinition where the value will be an 'object' retrieved by the labda expression: 'Builders<TModel>.Filter.Gt(xx.Key.FieldDefinition, xx.Value)'. So based on your response serializing the value part of a FilterDefinition could be an issue? Then again it was and is always possible to explicitly cast a string to a FieldDefinition and use it the same way (my current workaround) and the serialization of the object went fine which then indicates for me that it shouldn't affect the serialization part? But ofcourse i don't know the inner workings of the driver so i can overlook things. | ||
| Comment by Robert Stam [ 01/Feb/23 ] | ||
|
Thanks for your quick response. I'm glad you have a workaround you can use in the meantime. I do have a code change that addresses the mismatch between the actual type and the declared type (`ObjectId` vs `object` in your example). If the field definition is ONLY being used to figure out the field name, the change currently in code review will be fine. But if the field definition is also used to determine how to serialize values of type `TField` that probably won't work properly. For `Sort.Ascending` it is only the field name that matters, not the actual type. | ||
| Comment by DanikRaikhlin N/A [ 01/Feb/23 ] | ||
|
Thanks for taking a look! The example that i've given to reproduce the issue is an oversimplification of the actual scenario that we have. In our usecase at that point we don't know what the concrete class nor what the property actually is. We define the labda expression as a property accessor to get the actual value from a class but also using the same expression to create a FilterDefinition dynamically in a very generic way. So yes if i knew the concrete type and the concrete property type then yes ofcourse i could change 'object' to that specific type but that isn't possible for us in this scenario. However for us i've already build a workaround for to extract the property name from the expression and use that to create a FieldDefinition.
The decision on weather this is a bug or by design i'm leaving up to you. I'm just demonstrating a difference in rendering of queries between LINQ2 and LINQ3 provider. | ||
| Comment by Robert Stam [ 01/Feb/23 ] | ||
|
Actually, I'm no longer sure whether there is a bug here or not. When you use the `FieldDefinition<TDocument, TField>` class we expect that `TField` will accurately reflect the true type of the field. In your example, the true type of the field is `ObjectId`, but you are declaring the field definition to be `FieldDefinition<ConcreteClass, object>` and `object` is not the true type of the field. Note that your example would run correctly if you did either of the following two things: 1. Use `ObjectId` as the `TField` type:
Let me know if using one of these two "more correct" forms addresses your issue. Thanks! | ||
| Comment by Robert Stam [ 30/Jan/23 ] | ||
|
Thank you for reporting this issue. I am able to reproduce and have found a fix that will be in the next release. |