[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: File TestProject1.7z     PNG File image-2023-08-31-10-22-53-005.png    
Documentation Changes Summary:

1. What would you like to communicate to the user about this feature?
2. Would you like the user to see examples of the syntax and/or executable code and its output?
3. Which versions of the driver/connector does this apply to?


 Description   

Hi,
 
we have a regression in 
Builders<Class1>.Update.Set(x => x.Prop1.SubProp1, "someValue");
We get an exception (see below) in LinqProvider V3. V2 works fine.
 
Thanks!
 
Best Regards
Dimitri
 
MongoDB.Driver.Linq.ExpressionNotSupportedException: Expression not supported: ConvertChecked(Param_0.Prop1.SubProp1, String).
at MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToFilterTranslators.ToFilterFieldTranslators.ExpressionToFilterFieldTranslator.Translate(TranslationContext context, Expression expression)
at MongoDB.Driver.Linq.Linq3Implementation.LinqProviderAdapterV3.TranslateExpressionToField[TDocument,TField](Expression`1 expression, IBsonSerializer`1 documentSerializer, IBsonSerializerRegistry serializerRegistry, Boolean allowScalarValueForArrayField)
at MongoDB.Driver.ExpressionFieldDefinition`2.Render(IBsonSerializer`1 documentSerializer, IBsonSerializerRegistry serializerRegistry, LinqProvider linqProvider, Boolean allowScalarValueForArrayField)
at MongoDB.Driver.ExpressionFieldDefinition`2.Render(IBsonSerializer`1 documentSerializer, IBsonSerializerRegistry serializerRegistry, LinqProvider linqProvider)
at MongoDB.Driver.OperatorUpdateDefinition`2.Render(IBsonSerializer`1 documentSerializer, IBsonSerializerRegistry serializerRegistry, LinqProvider linqProvider)
at MongoDB.Driver.MongoCollectionImpl`1.ConvertWriteModelToWriteRequest(WriteModel`1 model, Int32 index)



 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:

Builders<Class1>.Update.Set(x => x.Prop1.SubProp1, "someValue"); 

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.
There are multiple problems with omitting the expression parts that can not be translated to MQL, but maybe Convert is a special case.
Thank you for providing the use case.

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
 

class Member
{
    public string Name { get; set; }
    public Enrollment Enrollment { get; set; }
}
 
class Monthly : Enrollment 
{
    public DateTime StartDate { get; set; }
}
 
Builders<Member>.Update.CurrentDate(x => ((Monthly)x.Enrollment).StartDate)   

 

Comment by Shiya Kohn [ 24/Aug/23 ]

It works if you replace `ConvertChecked` with `Convert`.

Comment by Boris Dogadov [ 24/Aug/23 ]

Hi dimitri.kroo@baramundi.com 

Could you please elaborate on need for changing the type of the property in BuildPropAccessor?
ConvertChecked was ignored by LINQ2. In LINQ3 the all queries are executed on server side, so there is not real translation for changing the types.

Wondering whether the following approach would address your use case:

private static Expression<Func<PlaceHeader, T>> BuildPropAccessor<T>(string propName)
{
   var parameter = Expression.Parameter(typeof(PlaceHeader));
   var memberExpression = Expression.Property(parameter, propName);
   return Expression.Lambda<Func<PlaceHeader, T>>(memberExpression, parameter);
}

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)
       

{             var param = Expression.Parameter(typeof(ParentClass));             var body = Expression.ConvertChecked(Expression.Property(Expression.Property(param, nameof(ParentClass.Child)), propName), typeof(TPROP));             var prop = Expression.Lambda<Func<ParentClass, TPROP>>(body, param);             return prop;         }
Comment by Boris Dogadov [ 16/Aug/23 ]

Thanks dimitri.kroo@baramundi.com for your question.

We have failed to reproduce this behaviour.
Could you please share a simple repro demonstrating this failure?
We would also need the types for Class1 and its nested types.

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.

Generated at Wed Feb 07 21:49:16 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.