[rc2] Fix issues with primitive collections inside JSON complex types

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.ExecuteUpdate.cs

@@ -89,7 +89,7 @@ public partial class RelationalQueryableMethodTranslatingExpressionVisitor
                     RemapLambdaBody(source, firstPropertySelector).UnwrapTypeConversion(out _),
                     RelationalDependencies.Model,
                     out var baseExpression)
-                || baseExpression.UnwrapTypeConversion(out _) is not StructuralTypeShaperExpression shaper)
+                || _sqlTranslator.TranslateProjection(baseExpression) is not StructuralTypeShaperExpression shaper)
             {
                 AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(firstPropertySelector));
                 return null;
@@ -270,14 +270,65 @@ protected virtual bool TryTranslateSetters(
             var (propertySelector, valueSelector) = setter;
             var propertySelectorBody = RemapLambdaBody(source, propertySelector).UnwrapTypeConversion(out _);
 
-            // The top-most node on the property selector must be a member access; chop it off to get the base expression and member.
-            // We'll bind the member manually below, so as to get the IPropertyBase it represents - that's important for later.
-            if (!IsMemberAccess(propertySelectorBody, QueryCompilationContext.Model, out var baseExpression, out var member)
-                || !_sqlTranslator.TryBindMember(
-                    _sqlTranslator.Visit(baseExpression), member, out var target, out var targetProperty))
+            // First, translate the property selector of the selector (left-hand side).
+            // The top-most node on the property selector must generally be a member access which corresponds to the property
+            // we will be updating (we also support ElementAt over that for indexing into collections).
+            // Later down we'll need the model property, so we can't just translate the entire expression as-is; we instead
+            // specifically recognize member access expressions and manually bind the member access to get the property out.
+            Expression target;
+            IPropertyBase targetProperty;
+
+            switch (propertySelectorBody)
             {
-                AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertySelector.Print()));
-                return false;
+                // Regular member access (common case)
+                case var _ when TryTranslateMemberAccess(propertySelectorBody, out var tempTarget, out var tempTargetProperty):
+                    target = tempTarget;
+                    targetProperty = tempTargetProperty;
+                    break;
+
+                // Index access inside collections - we get a property selector with <collection property>.AsQueryable().ElementAt(6)
+                case MethodCallExpression
+                {
+                    Method: { Name: nameof(Enumerable.ElementAt), IsGenericMethod: true } elementAtMethod,
+                    Arguments:
+                    [
+                        MethodCallExpression
+                        {
+                            Method: { Name: nameof(Queryable.AsQueryable), IsGenericMethod: true } asQueryableMethod,
+                            Arguments: [var elementAtSource]
+                        },
+                        _
+                    ]
+                } methodCall
+                    when elementAtMethod.GetGenericMethodDefinition() == QueryableMethods.ElementAt
+                        && asQueryableMethod.GetGenericMethodDefinition() == QueryableMethods.AsQueryable
+                        && TryTranslateMemberAccess(elementAtSource, out var translatedMember, out var tempTargetProperty)
+                        && Visit(methodCall) is Expression tempTarget:
+                    target = tempTarget;
+                    targetProperty = tempTargetProperty;
+                    break;
+
+                default:
+                    AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertySelector.Print()));
+                    return false;
+
+                bool TryTranslateMemberAccess(
+                    Expression expression,
+                    [NotNullWhen(true)] out Expression? translation,
+                    [NotNullWhen(true)] out IPropertyBase? property)
+                {
+                    if (IsMemberAccess(expression, QueryCompilationContext.Model, out var baseExpression, out var member)
+                        && _sqlTranslator.TryBindMember(_sqlTranslator.Visit(baseExpression), member, out var target, out var targetProperty))
+                    {
+                        translation = target;
+                        property = targetProperty;
+                        return true;
+                    }
+
+                    translation = null;
+                    property = null;
+                    return false;
+                }
             }
 
             if (targetProperty.DeclaringType is IEntityType entityType && entityType.IsMappedToJson())
@@ -291,7 +342,15 @@ protected virtual bool TryTranslateSetters(
             // Call TranslateProjection to unwrap it (need to look into getting rid StructuralTypeReferenceExpression altogether).
             if (target is not CollectionResultExpression)
             {
-                target = _sqlTranslator.TranslateProjection(target);
+                if (_sqlTranslator.TranslateProjection(target) is { } unwrappedTarget)
+                {
+                    target = unwrappedTarget;
+                }
+                else
+                {
+                    AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertySelector.Print()));
+                    return false;
+                }
             }
 
             switch (target)

src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs

@@ -443,8 +443,8 @@ bool TryTranslate(
             // JSON_VALUE within JSON_VALUE.
             var (json, path) = jsonArrayColumn is JsonScalarExpression innerJsonScalarExpression
                 ? (innerJsonScalarExpression.Json,
-                    innerJsonScalarExpression.Path.Append(new PathSegment(translatedIndex)).ToArray())
-                : (jsonArrayColumn, new PathSegment[] { new(translatedIndex) });
+                    innerJsonScalarExpression.Path.Append(new(translatedIndex)).ToArray())
+                : (jsonArrayColumn, [new(translatedIndex)]);
 
             var translation = new JsonScalarExpression(
                 json,
@@ -581,10 +581,11 @@ protected override bool TryTranslateSetters(
         SqlExpression value,
         ref SqlExpression? existingSetterValue)
     {
-        var (jsonColumn, path) = target switch
+        var (jsonColumn, path, isJsonScalar) = target switch
         {
-            JsonScalarExpression j => ((ColumnExpression)j.Json, j.Path),
-            JsonQueryExpression j => (j.JsonColumn, j.Path),
+            JsonScalarExpression { TypeMapping.ElementTypeMapping: null } j => ((ColumnExpression)j.Json, j.Path, true),
+            JsonScalarExpression { TypeMapping.ElementTypeMapping: not null } j => ((ColumnExpression)j.Json, j.Path, false),
+            JsonQueryExpression j => (j.JsonColumn, j.Path, false),
 
             _ => throw new UnreachableException(),
         };
@@ -612,14 +613,14 @@ protected override bool TryTranslateSetters(
                     // as a constant argument; it will be unpacked and handled in SQL generation.
                     _sqlExpressionFactory.Constant(path, RelationalTypeMapping.NullMapping),
 
-                // If an inline JSON object (complex type) is being assigned, it would be rendered here as a simple string:
-                // [column].modify('$.foo', '{ "x": 8 }')
-                // Since it's untyped, modify would treat is as a string rather than a JSON object, and insert it as such into
-                // the enclosing object, escaping all the special JSON characters - that's not what we want.
-                // We add a cast to JSON to have it interpreted as a JSON object.
-                value is SqlConstantExpression { TypeMapping.StoreType: "json" }
-                    ? _sqlExpressionFactory.Convert(value, value.Type, _typeMappingSource.FindMapping("json")!)
-                    : value
+                    // If an inline JSON object (complex type) is being assigned, it would be rendered here as a simple string:
+                    // [column].modify('$.foo', '{ "x": 8 }')
+                    // Since it's untyped, modify would treat is as a string rather than a JSON object, and insert it as such into
+                    // the enclosing object, escaping all the special JSON characters - that's not what we want.
+                    // We add a cast to JSON to have it interpreted as a JSON object.
+                    value is SqlConstantExpression { TypeMapping.StoreType: "json" }
+                        ? _sqlExpressionFactory.Convert(value, value.Type, _typeMappingSource.FindMapping("json")!)
+                        : value
                 ],
                 nullable: true,
                 instancePropagatesNullability: true,
@@ -642,11 +643,12 @@ protected override bool TryTranslateSetters(
                 // as a constant argument; it will be unpacked and handled in SQL generation.
                 _sqlExpressionFactory.Constant(path, RelationalTypeMapping.NullMapping),
                 // JSON_MODIFY by default assumes nvarchar(max) is text and escapes it.
-                // In order to set a JSON fragment (for nested JSON objects), we need to wrap the JSON text with JSON_QUERY(), which makes
-                // JSON_MODIFY understand that it's JSON content and prevents escaping.
-                target is JsonQueryExpression && value is not JsonScalarExpression
-                    ? _sqlExpressionFactory.Function("JSON_QUERY", [value], nullable: true, argumentsPropagateNullability: [true], typeof(string), value.TypeMapping)
-                    : value
+                // In order to set a JSON fragment (for nested JSON objects, primitive collections), we need to wrap the JSON text with
+                // JSON_QUERY(), which makes JSON_MODIFY understand that it's JSON content and prevents escaping.
+                // If the value expression happens to be JsonScalarExpression (i.e. another JSON property), we don't need to do this.
+                isJsonScalar || value is JsonScalarExpression
+                    ? value
+                    : _sqlExpressionFactory.Function("JSON_QUERY", [value], nullable: true, argumentsPropagateNullability: [true], typeof(string), value.TypeMapping)
             ],
             nullable: true,
             argumentsPropagateNullability: [true, true, true],

src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs

@@ -554,9 +554,16 @@ bool TryTranslate(
             if (shaperExpression is ProjectionBindingExpression projectionBindingExpression
                 && selectExpression.GetProjection(projectionBindingExpression) is ColumnExpression projectionColumn)
             {
+                // If the inner expression happens to itself be a JsonScalarExpression, simply append the two paths to avoid creating
+                // JSON_VALUE within JSON_VALUE.
+                var (json, path) = jsonArrayColumn is JsonScalarExpression innerJsonScalarExpression
+                    ? (innerJsonScalarExpression.Json,
+                        innerJsonScalarExpression.Path.Append(new(translatedIndex)).ToArray())
+                    : (jsonArrayColumn, [new(translatedIndex)]);
+
                 SqlExpression translation = new JsonScalarExpression(
-                    jsonArrayColumn,
-                    [new PathSegment(translatedIndex)],
+                    json,
+                    path,
                     projectionColumn.Type,
                     projectionColumn.TypeMapping,
                     projectionColumn.IsNullable);
@@ -642,10 +649,11 @@ protected override bool TrySerializeScalarToJson(
         SqlExpression value,
         ref SqlExpression? existingSetterValue)
     {
-        var (jsonColumn, path) = target switch
+        var (jsonColumn, path, isJsonScalar) = target switch
         {
-            JsonScalarExpression j => ((ColumnExpression)j.Json, j.Path),
-            JsonQueryExpression j => (j.JsonColumn, j.Path),
+            JsonScalarExpression { TypeMapping.ElementTypeMapping: null } j => ((ColumnExpression)j.Json, j.Path, true),
+            JsonScalarExpression { TypeMapping.ElementTypeMapping: not null } j => ((ColumnExpression)j.Json, j.Path, false),
+            JsonQueryExpression j => (j.JsonColumn, j.Path, false),
 
             _ => throw new UnreachableException(),
         };
@@ -661,9 +669,9 @@ protected override bool TrySerializeScalarToJson(
                 // json_set by default assumes text and escapes it.
                 // In order to set a JSON fragment (for nested JSON objects), we need to wrap the JSON text with json(), which makes
                 // json_set understand that it's JSON content and prevents escaping.
-                target is JsonQueryExpression
-                    ? _sqlExpressionFactory.Function("json", [value], nullable: true, argumentsPropagateNullability: [true], typeof(string), value.TypeMapping)
-                    : value
+                isJsonScalar
+                    ? value
+                    : _sqlExpressionFactory.Function("json", [value], nullable: true, argumentsPropagateNullability: [true], typeof(string), value.TypeMapping)
             ],
             nullable: true,
             argumentsPropagateNullability: [true, true, true],

src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Runtime.CompilerServices;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using ExpressionExtensions = Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions;
@@ -582,13 +583,15 @@ when QueryableMethods.IsSumWithSelector(method):
                             thenInclude: false,
                             setLoaded: false);
 
-                    // Handled by relational/provider even though method is on `EntityFrameworkQueryableExtensions`
-                    case nameof(EntityFrameworkQueryableExtensions.ExecuteDelete):
-                    case nameof(EntityFrameworkQueryableExtensions.ExecuteDeleteAsync):
-                    case nameof(EntityFrameworkQueryableExtensions.ExecuteUpdate):
-                    case nameof(EntityFrameworkQueryableExtensions.ExecuteUpdateAsync):
+                    // Nothing to do (no arguments), the generic unknown method logic handles the source.
+                    case nameof(EntityFrameworkQueryableExtensions.ExecuteDelete)
+                        when genericMethod == EntityFrameworkQueryableExtensions.ExecuteDeleteMethodInfo:
                         return ProcessUnknownMethod(methodCallExpression);
 
+                    case nameof(EntityFrameworkQueryableExtensions.ExecuteUpdate)
+                        when genericMethod == EntityFrameworkQueryableExtensions.ExecuteUpdateMethodInfo:
+                        return ProcessExecuteUpdate(source, genericMethod, methodCallExpression.Arguments[1]);
+
                     case nameof(Queryable.GroupBy)
                         when genericMethod == QueryableMethods.GroupByWithKeySelector:
                         return ProcessGroupBy(
@@ -1057,6 +1060,47 @@ private NavigationExpansionExpression ProcessElementAt(
         return source;
     }
 
+    private Expression ProcessExecuteUpdate(NavigationExpansionExpression source, MethodInfo method, Expression setters)
+    {
+        source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
+
+        NewArrayExpression settersArray;
+        switch (setters)
+        {
+            case NewArrayExpression e:
+                settersArray = e;
+                break;
+            // Empty setters - nothing to do here (we'll throw an error later in the query pipeline)
+            case ConstantExpression { Value: ITuple[] { Length: 0 } }:
+                return Expression.Call(method.MakeGenericMethod(source.SourceElementType), source.Source, setters);
+            default:
+                throw new UnreachableException();
+        }
+
+        var newSetters = new Expression[settersArray.Expressions.Count];
+
+        for (var i = 0; i < settersArray.Expressions.Count; i++)
+        {
+            var setter = (NewExpression)settersArray.Expressions[i];
+
+            Check.DebugAssert(setter.Type == typeof(Tuple<Delegate, object>));
+
+            var (propertySelector, valueSelector) = ((LambdaExpression)setter.Arguments[0], setter.Arguments[1]);
+
+            var processedPropertySelector = ProcessLambdaExpression(source, propertySelector);
+            var processedValueSelector = valueSelector is LambdaExpression valueSelectorLambda
+                ? ProcessLambdaExpression(source, valueSelectorLambda)
+                : Visit(valueSelector);
+
+            newSetters[i] = Expression.New(setter.Constructor!, processedPropertySelector, processedValueSelector);
+        }
+
+        return Expression.Call(
+            method.MakeGenericMethod(source.SourceElementType),
+            source.Source,
+            Expression.NewArrayInit(typeof(Tuple<Delegate, object>), newSetters));
+    }
+
     // This returns Expression since it can also return a deferred GroupBy operation
     private Expression ProcessGroupBy(
         NavigationExpansionExpression source,
@@ -2316,6 +2360,7 @@ private static Expression SnapshotExpression(Expression selector)
         {
             EntityReference entityReference => entityReference,
             ComplexTypeReference complexTypeReference => complexTypeReference,
+            ComplexPropertyReference complexPropertyReference => complexPropertyReference.ComplexTypeReference,
             NavigationTreeExpression navigationTreeExpression => UnwrapStructuralTypeReference(navigationTreeExpression.Value),
             NavigationExpansionExpression navigationExpansionExpression
                 when navigationExpansionExpression.CardinalityReducingGenericMethodInfo is not null