Fixes to complex values types

Part of dotnet#31376 and dotnet#36296
Continues dotnet#36557

Author: roji

src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs

@@ -320,8 +320,13 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
                     _projectionMapping[_projectionMembers.Peek()] = jsonQueryExpression;
 
+#pragma warning disable EF1001
                     return shaper.Update(
-                        new ProjectionBindingExpression(_selectExpression, _projectionMembers.Peek(), typeof(ValueBuffer)));
+                        new ProjectionBindingExpression(_selectExpression, _projectionMembers.Peek(), typeof(ValueBuffer)))
+                            // This is to handle have correct type for the shaper expression. It is later fixed in MatchTypes.
+                            // This mirrors for structural types what we do for scalars.
+                            .MakeClrTypeNullable();
+#pragma warning restore EF1001
                 }
 
                 if (shaper.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression)
@@ -342,13 +347,23 @@ protected override Expression VisitExtension(Expression extensionExpression)
                         {
                             var projectionBinding = AddClientProjection(jsonQuery, typeof(ValueBuffer));
 
-                            return shaper.Update(projectionBinding);
+#pragma warning disable EF1001
+                            return shaper.Update(projectionBinding)
+                                // This is to handle have correct type for the shaper expression. It is later fixed in MatchTypes.
+                                // This mirrors for structural types what we do for scalars.
+                                .MakeClrTypeNullable();
+#pragma warning restore EF1001
                         }
 
                         _projectionMapping[_projectionMembers.Peek()] = jsonQuery;
 
+#pragma warning disable EF1001
                         return shaper.Update(
-                            new ProjectionBindingExpression(_selectExpression, _projectionMembers.Peek(), typeof(ValueBuffer)));
+                            new ProjectionBindingExpression(_selectExpression, _projectionMembers.Peek(), typeof(ValueBuffer)))
+                            // This is to handle have correct type for the shaper expression. It is later fixed in MatchTypes.
+                            // This mirrors for structural types what we do for scalars.
+                            .MakeClrTypeNullable();
+#pragma warning restore EF1001
                     }
 
                     projection = (StructuralTypeProjectionExpression)projection2;
@@ -366,14 +381,19 @@ protected override Expression VisitExtension(Expression extensionExpression)
                         _projectionBindingCache[projection] = entityProjectionBinding;
                     }
 
-                    return shaper.Update(entityProjectionBinding);
+#pragma warning disable EF1001
+                    return shaper.Update(entityProjectionBinding)
+                        // This is to handle have correct type for the shaper expression. It is later fixed in MatchTypes.
+                        // This mirrors for structural types what we do for scalars.
+                        .MakeClrTypeNullable();
+#pragma warning restore EF1001
                 }
 
                 _projectionMapping[_projectionMembers.Peek()] = projection;
 
+#pragma warning disable EF1001
                 return shaper
                     .Update(new ProjectionBindingExpression(_selectExpression, _projectionMembers.Peek(), typeof(ValueBuffer)))
-#pragma warning disable EF1001
                     // This is to handle have correct type for the shaper expression. It is later fixed in MatchTypes.
                     // This mirrors for structural types what we do for scalars.
                     .MakeClrTypeNullable();

src/EFCore.Relational/Query/Internal/Translators/NullableMemberTranslator.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 
 namespace Microsoft.EntityFrameworkCore.Query;
@@ -21,7 +20,6 @@ public RelationalMemberTranslatorProvider(RelationalMemberTranslatorProviderDepe
         Dependencies = dependencies;
 
         _plugins.AddRange(dependencies.Plugins.SelectMany(p => p.Translators));
-        _translators.AddRange([new NullableMemberTranslator(dependencies.SqlExpressionFactory)]);
     }
 
     /// <summary>

src/EFCore.Relational/Query/RelationalMemberTranslatorProvider.cs

@@ -628,6 +628,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
                                 var shaperResult = CreateJsonShapers(
                                     shaper.StructuralType,
+                                    shaper.Type,
                                     shaper.IsNullable,
                                     jsonReaderDataVariable,
                                     keyValuesParameter,
@@ -674,8 +675,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
                                         childProjectionInfo.Navigation.TargetEntityType,
                                         childProjectionInfo.Navigation.IsCollection);
 
+                                    var targetEntityType = childProjectionInfo.Navigation.TargetEntityType;
                                     var shaperResult = CreateJsonShapers(
-                                        childProjectionInfo.Navigation.TargetEntityType,
+                                        targetEntityType,
+                                        targetEntityType.ClrType,
                                         nullable: true,
                                         jsonReaderDataVariable,
                                         keyValuesParameter,
@@ -784,6 +787,7 @@ when GetProjectionIndex(projectionBindingExpression) is JsonProjectionInfo jsonP
 
                     var shaperResult = CreateJsonShapers(
                         relatedStructuralType,
+                        relationship.ClrType,
                         nullable: true,
                         jsonReaderDataVariable,
                         keyValuesParameter,
@@ -1151,8 +1155,10 @@ when GetProjectionIndex(projectionBindingExpression) is JsonProjectionInfo jsonP
                                 includeExpression.Navigation.TargetEntityType,
                                 includeExpression.Navigation.IsCollection);
 
+                            var targetEntityType = includeExpression.Navigation.TargetEntityType;
                             var shaperResult = CreateJsonShapers(
-                                includeExpression.Navigation.TargetEntityType,
+                                targetEntityType,
+                                targetEntityType.ClrType,
                                 nullable: true,
                                 jsonReaderDataVariable,
                                 keyValuesParameter,
@@ -1563,6 +1569,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
 
         private Expression CreateJsonShapers(
             ITypeBase structuralType,
+            Type clrType,
             bool nullable,
             ParameterExpression jsonReaderDataParameter,
             Expression? keyValuesParameter,
@@ -1628,6 +1635,7 @@ private Expression CreateJsonShapers(
 
                 var innerShaper = CreateJsonShapers(
                     relatedStructuralType,
+                    nestedRelationship.ClrType,
                     nullable || isRelationshipNullable,
                     jsonReaderDataShaperLambdaParameter,
                     keyValuesShaperLambdaParameter,
@@ -1847,15 +1855,13 @@ private Expression CreateJsonShapers(
                 return materializeJsonEntityCollectionMethodCall;
             }
 
-
             // Return the materializer for this JSON object, including null checks which would return null.
             MethodInfo method;
 
-            if (relationship is not null && Nullable.GetUnderlyingType(relationship.ClrType) is { } underlyingType)
+            if (Nullable.GetUnderlyingType(clrType) is { } underlyingType)
             {
-                // The association property into which we're assigning has a nullable value type, so generate
-                // a materializer that returns that nullable value type (note that the shaperLambda that
-                // we pass itself always returns a non-nullable value (the null checks are outside of it.))
+                // We need to project out a nullable value type. Note that the shaperLambda that we pass itself always returns a
+                // non-nullable value (the null checks are outside of it.))
                 Check.DebugAssert(nullable, "On non-nullable relationship but the relationship's ClrType is Nullable<T>");
                 Check.DebugAssert(underlyingType == structuralType.ClrType);
 
@@ -2538,6 +2544,7 @@ internal void ProcessTopLevelComplexJsonProperties(
 
                         var shaperResult = CreateJsonShapers(
                             complexType,
+                            complexProperty.ClrType,
                             nullable: shaper.IsNullable || complexProperty.IsNullable,
                             jsonReaderDataVariable,
                             keyValuesParameter: null, // For owned entities only

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

@@ -369,7 +369,7 @@ SqlExpression Process(Expression expression)
                                 => new JsonScalarExpression(
                                     jsonQuery.JsonColumn,
                                     jsonQuery.Path,
-                                    jsonQuery.Type,
+                                    jsonQuery.Type.UnwrapNullableType(),
                                     jsonQuery.JsonColumn.TypeMapping,
                                     jsonQuery.IsNullable),
 
@@ -378,7 +378,7 @@ SqlExpression Process(Expression expression)
                                 => new JsonScalarExpression(
                                     jsonQuery.JsonColumn,
                                     jsonQuery.Path,
-                                    jsonQuery.Type,
+                                    jsonQuery.Type.UnwrapNullableType(),
                                     jsonQuery.JsonColumn.TypeMapping,
                                     jsonQuery.IsNullable),
 

src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.StructuralEquality.cs

@@ -635,18 +635,44 @@ protected override Expression VisitMember(MemberExpression memberExpression)
                 Expression.Condition(
                     cond.Test,
                     Expression.MakeMemberAccess(cond.IfTrue, memberExpression.Member),
-                    Expression.MakeMemberAccess(cond.IfFalse, memberExpression.Member)
-                ));
+                    Expression.MakeMemberAccess(cond.IfFalse, memberExpression.Member)));
         }
 
-        var innerExpression = Visit(memberExpression.Expression);
+        var inner = Visit(memberExpression.Expression);
 
-        return TryBindMember(innerExpression, MemberIdentity.Create(memberExpression.Member), out var expression)
-            ? expression
-            : (TranslationFailed(memberExpression.Expression, innerExpression, out var sqlInnerExpression)
+        var member = memberExpression.Member;
+
+        // Try binding the member to a property on the structural type
+        if (TryBindMember(inner, MemberIdentity.Create(member), out var expression))
+        {
+            return expression;
+        }
+
+        // We handle translations for Nullable<> members here.
+        // These can't be handled in regular IMemberTranslators, since those only support scalars (SqlExpressions);
+        // but we also need to handle nullable value complex types.
+        if (member.DeclaringType?.IsGenericType == true
+            && member.DeclaringType.GetGenericTypeDefinition() == typeof(Nullable<>)
+            && inner is not null)
+        {
+            switch (member.Name)
+            {
+                case nameof(Nullable<>.Value):
+                    return inner;
+                case nameof(Nullable<>.HasValue) when inner is SqlExpression sqlInner:
+                    return _sqlExpressionFactory.IsNotNull(sqlInner);
+                case nameof(Nullable<>.HasValue)
+                    when inner is StructuralTypeReferenceExpression
+                        && TryRewriteStructuralTypeEquality(
+                            ExpressionType.NotEqual, inner, new SqlConstantExpression(null, memberExpression.Expression!.Type, null), equalsMethod: false, out var result):
+                    return result;
+            }
+        }
+
+        return (TranslationFailed(memberExpression.Expression, inner, out var sqlInnerExpression)
                 ? QueryCompilationContext.NotTranslatedExpression
                 : Dependencies.MemberTranslatorProvider.Translate(
-                    sqlInnerExpression, memberExpression.Member, memberExpression.Type, _queryCompilationContext.Logger))
+                    sqlInnerExpression, member, memberExpression.Type, _queryCompilationContext.Logger))
             ?? QueryCompilationContext.NotTranslatedExpression;
     }
 
@@ -1692,7 +1718,7 @@ private static bool CanEvaluate(Expression expression)
     private static bool IsNullSqlConstantExpression(Expression expression)
         => expression is SqlConstantExpression { Value: null };
 
-    [DebuggerStepThrough]
+    // [DebuggerStepThrough]
     private static bool TranslationFailed(Expression? original, Expression? translation, out SqlExpression? castTranslation)
     {
         if (original != null

src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

@@ -187,13 +187,13 @@ public override StructuralTypeShaperExpression Update(Expression valueBufferExpr
             : this;
 
     /// <inheritdoc />
-    public override StructuralTypeShaperExpression MakeClrTypeNullable()
+    public override RelationalStructuralTypeShaperExpression MakeClrTypeNullable()
         => Type != Type.MakeNullable()
             ? new RelationalStructuralTypeShaperExpression(StructuralType, ValueBufferExpression, IsNullable, MaterializationCondition, Type.MakeNullable())
             : this;
 
     /// <inheritdoc />
-    public override StructuralTypeShaperExpression MakeClrTypeNonNullable()
+    public override RelationalStructuralTypeShaperExpression MakeClrTypeNonNullable()
         => Type != Type.UnwrapNullableType()
             ? new RelationalStructuralTypeShaperExpression(StructuralType, ValueBufferExpression, IsNullable, MaterializationCondition, Type.UnwrapNullableType())
             : this;

src/EFCore.Relational/Query/RelationalStructuralTypeShaperExpression.cs

@@ -1,12 +1,11 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Xunit.Sdk;
+using Microsoft.EntityFrameworkCore.Query.Relationships.ComplexProperties;
 
 namespace Microsoft.EntityFrameworkCore.Query.Relationships.ComplexTableSplitting;
 
-public abstract class ComplexTableSplittingProjectionRelationalTestBase<TFixture>
-    : RelationshipsProjectionTestBase<TFixture>
+public abstract class ComplexTableSplittingProjectionRelationalTestBase<TFixture> : ComplexPropertiesProjectionTestBase<TFixture>
     where TFixture : ComplexTableSplittingRelationalFixtureBase, new()
 {
     public ComplexTableSplittingProjectionRelationalTestBase(TFixture fixture, ITestOutputHelper testOutputHelper)

test/EFCore.Relational.Specification.Tests/Query/Relationships/ComplexTableSplitting/ComplexTableSplittingProjectionRelationalTestBase.cs

@@ -5,4 +5,23 @@ namespace Microsoft.EntityFrameworkCore.Query.Relationships.ComplexProperties;
 
 public abstract class ComplexPropertiesMiscellaneousTestBase<TFixture>(TFixture fixture)
     : RelationshipsMiscellaneousTestBase<TFixture>(fixture)
-    where TFixture : ComplexPropertiesFixtureBase, new();
+    where TFixture : ComplexPropertiesFixtureBase, new()
+{
+    #region Value types
+
+    [ConditionalFact]
+    public virtual Task Where_property_on_non_nullable_value_type()
+        => AssertQuery(ss => ss.Set<ValueRootEntity>().Where(e => e.RequiredRelated.Int == 8));
+
+    [ConditionalFact]
+    public virtual Task Where_property_on_nullable_value_type_Value()
+        => AssertQuery(
+            ss => ss.Set<ValueRootEntity>().Where(e => e.OptionalRelated!.Value.Int == 8),
+            ss => ss.Set<ValueRootEntity>().Where(e => e.OptionalRelated.HasValue && e.OptionalRelated!.Value.Int == 8));
+
+    [ConditionalFact]
+    public virtual Task Where_HasValue_on_nullable_value_type()
+        => AssertQuery(ss => ss.Set<ValueRootEntity>().Where(e => e.OptionalRelated.HasValue));
+
+    #endregion Value types
+}

test/EFCore.Specification.Tests/Query/Relationships/ComplexProperties/ComplexPropertiesMiscellaneousTestBase.cs

@@ -16,5 +16,32 @@ public virtual Task Select_root_with_value_types(QueryTrackingBehavior queryTrac
             ss => ss.Set<ValueRootEntity>(),
             queryTrackingBehavior: queryTrackingBehavior);
 
+
+    [ConditionalTheory]
+    [MemberData(nameof(TrackingData))]
+    public virtual Task Select_non_nullable_value_type(QueryTrackingBehavior queryTrackingBehavior)
+        => AssertQuery(
+            ss => ss.Set<ValueRootEntity>().OrderBy(e => e.Id).Select(x => x.RequiredRelated),
+            assertOrder: true,
+            queryTrackingBehavior: queryTrackingBehavior);
+
+
+    [ConditionalTheory]
+    [MemberData(nameof(TrackingData))]
+    public virtual Task Select_nullable_value_type(QueryTrackingBehavior queryTrackingBehavior)
+        => AssertQuery(
+            ss => ss.Set<ValueRootEntity>().OrderBy(e => e.Id).Select(x => x.OptionalRelated),
+            assertOrder: true,
+            queryTrackingBehavior: queryTrackingBehavior);
+
+    [ConditionalTheory]
+    [MemberData(nameof(TrackingData))]
+    public virtual Task Select_nullable_value_type_with_Value(QueryTrackingBehavior queryTrackingBehavior)
+        => AssertQuery(
+            ss => ss.Set<ValueRootEntity>().OrderBy(e => e.Id).Select(x => x.OptionalRelated!.Value),
+            ss => ss.Set<ValueRootEntity>().OrderBy(e => e.Id).Select(x => x.OptionalRelated == null ? default : x.OptionalRelated!.Value),
+            assertOrder: true,
+            queryTrackingBehavior: queryTrackingBehavior);
+
     #endregion Value types
 }