[CSHARP-4754] Nested or subproperties in Update.Set don't work in V3 Created: 16/Aug/23 Updated: 27/Oct/23 Resolved: 10/Oct/23 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 2.21.0 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Blocker - P1 |
| Reporter: | Dimitri Kroo | Assignee: | Robert Stam |
| Resolution: | Works as Designed | Votes: | 3 |
| Labels: | LINQ3 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
| Documentation Changes Summary: | 1. What would you like to communicate to the user about this feature? |
| Description |
|
Hi, |
| Comments |
| Comment by Robert Stam [ 10/Oct/23 ] | ||||||||||||
|
Thanks for confirming that using Convert works for you. | ||||||||||||
| Comment by Dimitri Kroo [ 05/Oct/23 ] | ||||||||||||
|
Hi Robert. thank you. You are right, I haven't found any reason to use ConvertChecked.
Best regards Dimitri
| ||||||||||||
| Comment by Robert Stam [ 04/Oct/23 ] | ||||||||||||
|
I wrote a small test for this specifically:
and it works fine. It seems like your best course of action is to double check the code where you generate `Expression`s and make sure you generate the same `Expressions` that the compiler generates. | ||||||||||||
| Comment by Robert Stam [ 04/Oct/23 ] | ||||||||||||
|
I started to look into reproducing this but I find your attached TestProject1 rather complex to work with. Any chance you could simplify the reproduction to a single file that is short as possible? Since you are generating the `Expression`s yourself as a work around have you tried using `Convert` instead of `ConvertChecked`? Our tests are focused on supporting actual `Expression`s generated by the compiler itself. Once you start generating your own `Expression`s you risk creating expressions that the compiler might never generate, and which we might not know what to do with. | ||||||||||||
| Comment by Dimitri Kroo [ 31/Aug/23 ] | ||||||||||||
|
Hi, sorry for late reponse... was on vacation Thank you both for workarounds! Do you plan to support "ConvertChecked" in next minor version? P.S. I have second Bug in V3: CSHARP-4762 Expression as variable in .Select() doesn't work with Linq V3 - MongoDB Jira So I would test both with fixes/workarounds to be able to switch to V3 completely.
| ||||||||||||
| Comment by Shiya Kohn [ 24/Aug/23 ] | ||||||||||||
|
Thanks for the quick response boris.dogadov@mongodb.com I'm not suggesting paths don't get translated at all, I'm suggesting that expressions that are explicitly used with the intention is only to get the name of a path for a simple update should have a bit more liberty on what they can contain. | ||||||||||||
| Comment by Boris Dogadov [ 24/Aug/23 ] | ||||||||||||
|
Hi skohn@goflow.com, Paths are translated as they are part of MQL. | ||||||||||||
| Comment by Shiya Kohn [ 24/Aug/23 ] | ||||||||||||
|
boris.dogadov@mongodb.com why does V3 attempt to translate path-only expressions. I understand that in V3 more query-related expressions are optimized to be server-side aggregations, but in terms of getting the path for an update definition only, why should it be restricted to the subset of expression available on the aggregation end? What if we have a need to update a field with a different type, why should casting not just be ignored in update definitions?
As far as a valid need for this, we have code paths where we need to update nested child properties on a property that is typed as the base.
Example
| ||||||||||||
| Comment by Shiya Kohn [ 24/Aug/23 ] | ||||||||||||
|
It works if you replace `ConvertChecked` with `Convert`. | ||||||||||||
| Comment by Boris Dogadov [ 24/Aug/23 ] | ||||||||||||
|
Could you please elaborate on need for changing the type of the property in BuildPropAccessor? Wondering whether the following approach would address your use case:
| ||||||||||||
| Comment by Boris Dogadov [ 18/Aug/23 ] | ||||||||||||
|
Thanks dimitri.kroo@baramundi.com We have reproduced this behavior, and will post further updates in this ticket. | ||||||||||||
| Comment by Dimitri Kroo [ 18/Aug/23 ] | ||||||||||||
|
Hi, thanks for checking! yeah, it is complicated in this case. I have attached a TestProject. private static readonly LinqProvider _linqProvider = LinqProvider.V3; => if you change it to V2 test will pass. The problem is that we use dynamic prop creation with this function: protected Expression<Func<ParentClass, TPROP>> BuildPropAccessor<TPROP>(string propName) | ||||||||||||
| Comment by Boris Dogadov [ 16/Aug/23 ] | ||||||||||||
|
Thanks dimitri.kroo@baramundi.com for your question. We have failed to reproduce this behaviour. | ||||||||||||
| Comment by PM Bot [ 16/Aug/23 ] | ||||||||||||
|
Hi dimitri.kroo@baramundi.com, thank you for reporting this issue! The team will look into it and get back to you soon. |