[release/7.0] Use correct provider type for nullable value type with …

…converter

Port of #29746
Fixes #29985

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -1393,8 +1393,6 @@ protected virtual void ValidateCompatible(
                     storeObject.DisplayName()));
         }
 
-        var typeMapping = property.GetRelationalTypeMapping();
-        var duplicateTypeMapping = duplicateProperty.GetRelationalTypeMapping();
         var currentTypeString = property.GetColumnType(storeObject);
         var previousTypeString = duplicateProperty.GetColumnType(storeObject);
         if (!string.Equals(currentTypeString, previousTypeString, StringComparison.OrdinalIgnoreCase))
@@ -1411,6 +1409,9 @@ protected virtual void ValidateCompatible(
                     currentTypeString));
         }
 
+        var typeMapping = property.GetRelationalTypeMapping();
+        var duplicateTypeMapping = duplicateProperty.GetRelationalTypeMapping();
+
         Type currentProviderType, previousProviderType;
         if (QuirkEnabled29531)
         {

src/EFCore.Relational/Metadata/Internal/ColumnBase.cs

@@ -12,6 +12,9 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
 public class ColumnBase<TColumnMappingBase> : Annotatable, IColumnBase
     where TColumnMappingBase : class, IColumnMappingBase
 {
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     private Type? _providerClrType;
 
     /// <summary>
@@ -84,7 +87,9 @@ public virtual Type ProviderClrType
             }
 
             var typeMapping = StoreTypeMapping;
-            var providerType = typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType;
+            var providerType = QuirkEnabled29985
+                ? typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType
+                : typeMapping.Converter?.ProviderClrType.UnwrapNullableType() ?? typeMapping.ClrType;
 
             return _providerClrType = providerType;
         }

src/EFCore.Relational/Update/Internal/ColumnAccessorsFactory.cs

@@ -13,6 +13,9 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal;
 /// </summary>
 public static class ColumnAccessorsFactory
 {
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -51,7 +54,7 @@ private static ColumnAccessors CreateGeneric<TColumn>(IColumn column)
 
                     var providerValue = entry.GetCurrentProviderValue(property);
                     if (providerValue == null
-                        && !typeof(TColumn).IsNullableType())
+                        && (!QuirkEnabled29985 || !typeof(TColumn).IsNullableType()))
                     {
                         return (value!, valueFound);
                     }
@@ -94,7 +97,7 @@ private static ColumnAccessors CreateGeneric<TColumn>(IColumn column)
 
                     var providerValue = entry.GetOriginalProviderValue(property);
                     if (providerValue == null
-                        && !typeof(TColumn).IsNullableType())
+                        && (!QuirkEnabled29985 || !typeof(TColumn).IsNullableType()))
                     {
                         return (value!, valueFound);
                     }

src/EFCore.Relational/Update/Internal/RowForeignKeyValueFactoryFactory.cs

@@ -14,6 +14,9 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal;
 /// </summary>
 public class RowForeignKeyValueFactoryFactory : IRowForeignKeyValueFactoryFactory
 {
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     private readonly IValueConverterSelector _valueConverterSelector;
 
     /// <summary>
@@ -53,30 +56,60 @@ private static IRowForeignKeyValueFactory CreateSimple<TKey, TForeignKey>(
         IValueConverterSelector valueConverterSelector)
         where TKey : notnull
     {
-        var dependentColumn = foreignKey.Columns.Single();
-        var dependentType = dependentColumn.ProviderClrType;
-        var principalType = foreignKey.PrincipalColumns.Single().ProviderClrType;
-        var columnAccessors = ((Column)dependentColumn).Accessors;
-
-        if (dependentType.IsNullableType()
-            && principalType.IsNullableType())
+        if (QuirkEnabled29985)
         {
-            return new SimpleFullyNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
-                foreignKey, dependentColumn, columnAccessors, valueConverterSelector);
-        }
+            var dependentColumn = foreignKey.Columns.Single();
+            var dependentType = dependentColumn.ProviderClrType;
+            var principalType = foreignKey.PrincipalColumns.Single().ProviderClrType;
+            var columnAccessors = ((Column)dependentColumn).Accessors;
+
+            if (dependentType.IsNullableType()
+                && principalType.IsNullableType())
+            {
+                return new SimpleFullyNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
+                    foreignKey, dependentColumn, columnAccessors, valueConverterSelector);
+            }
+
+            if (dependentType.IsNullableType())
+            {
+                return (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
+                    typeof(SimpleNullableRowForeignKeyValueFactory<,>).MakeGenericType(
+                        typeof(TKey), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors, valueConverterSelector)!;
+            }
 
-        if (dependentType.IsNullableType())
+            return principalType.IsNullableType()
+                ? (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
+                    typeof(SimpleNullablePrincipalRowForeignKeyValueFactory<,,>).MakeGenericType(
+                        typeof(TKey), typeof(TKey).UnwrapNullableType(), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors)!
+                : new SimpleNonNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
+                    foreignKey, dependentColumn, columnAccessors, valueConverterSelector);        }
+        else
         {
-            return (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
-                typeof(SimpleNullableRowForeignKeyValueFactory<,>).MakeGenericType(
-                    typeof(TKey), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors, valueConverterSelector)!;
-        }
+            var dependentColumn = foreignKey.Columns.First();
+            var principalColumn = foreignKey.PrincipalColumns.First();
+            var columnAccessors = ((Column)dependentColumn).Accessors;
+
+            if (principalColumn.ProviderClrType.IsNullableType()
+                || (dependentColumn.IsNullable
+                    && principalColumn.IsNullable))
+            {
+                return new SimpleFullyNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
+                    foreignKey, dependentColumn, columnAccessors, valueConverterSelector);
+            }
 
-        return principalType.IsNullableType()
-            ? (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
-                typeof(SimpleNullablePrincipalRowForeignKeyValueFactory<,,>).MakeGenericType(
-                    typeof(TKey), typeof(TKey).UnwrapNullableType(), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors)!
-            : new SimpleNonNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
-                foreignKey, dependentColumn, columnAccessors, valueConverterSelector);
+            if (dependentColumn.IsNullable)
+            {
+                return (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
+                    typeof(SimpleNullableRowForeignKeyValueFactory<,>).MakeGenericType(
+                        typeof(TKey), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors, valueConverterSelector)!;
+            }
+
+            return principalColumn.IsNullable
+                ? (IRowForeignKeyValueFactory<TKey>)Activator.CreateInstance(
+                    typeof(SimpleNullablePrincipalRowForeignKeyValueFactory<,,>).MakeGenericType(
+                        typeof(TKey), typeof(TKey).UnwrapNullableType(), typeof(TKey), typeof(TForeignKey)), foreignKey, dependentColumn, columnAccessors)!
+                : new SimpleNonNullableRowForeignKeyValueFactory<TKey, TForeignKey>(
+                    foreignKey, dependentColumn, columnAccessors, valueConverterSelector);
+        }
     }
 }

src/EFCore.Relational/Update/Internal/SimpleNullablePrincipalRowForeignKeyValueFactory.cs

@@ -13,7 +13,6 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal;
 /// </summary>
 public class SimpleNullablePrincipalRowForeignKeyValueFactory<TKey, TNonNullableKey, TForeignKey>
     : RowForeignKeyValueFactory<TKey, TForeignKey>
-    where TNonNullableKey : struct
 {
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore.Relational/Update/Internal/SimpleRowKeyValueFactory.cs

@@ -15,6 +15,9 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal;
 /// </summary>
 public class SimpleRowKeyValueFactory<TKey> : IRowKeyValueFactory<TKey>
 {
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     private readonly IUniqueConstraint _constraint;
     private readonly IColumn _column;
     private readonly ColumnAccessors _columnAccessors;
@@ -138,8 +141,36 @@ private sealed class NoNullsCustomEqualityComparer : IEqualityComparer<TKey>
 
         public NoNullsCustomEqualityComparer(ValueComparer comparer)
         {
-            _equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
-            _hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
+            if (QuirkEnabled29985)
+            {
+                _equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
+                _hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
+            }
+            else
+            {
+                var equals = comparer.EqualsExpression;
+                var getHashCode = comparer.HashCodeExpression;
+                var type = typeof(TKey);
+                if (type != comparer.Type)
+                {
+                    var newEqualsParam1 = Expression.Parameter(type, "v1");
+                    var newEqualsParam2 = Expression.Parameter(type, "v2");
+                    equals = Expression.Lambda(
+                        comparer.ExtractEqualsBody(
+                            Expression.Convert(newEqualsParam1, comparer.Type),
+                            Expression.Convert(newEqualsParam2, comparer.Type)),
+                        newEqualsParam1, newEqualsParam2);
+
+
+                    var newHashCodeParam = Expression.Parameter(type, "v");
+                    getHashCode = Expression.Lambda(
+                        comparer.ExtractHashCodeBody(
+                            Expression.Convert(newHashCodeParam, comparer.Type)),
+                        newHashCodeParam);
+                }
+
+                _equals = (Func<TKey?, TKey?, bool>)equals.Compile();
+                _hashCode = (Func<TKey, int>)getHashCode.Compile();            }
         }
 
         public bool Equals(TKey? x, TKey? y)

src/EFCore/ChangeTracking/Internal/CurrentValueComparerFactory.cs

@@ -13,6 +13,9 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 /// </summary>
 public class CurrentValueComparerFactory
 {
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -49,12 +52,25 @@ public virtual IComparer<IUpdateEntry> Create(IPropertyBase propertyBase)
                 var nonNullableProviderType = providerType.UnwrapNullableType();
                 if (IsGenericComparable(providerType, nonNullableProviderType))
                 {
-                    var comparerType = modelType.IsClass
-                        ? typeof(NullableClassCurrentProviderValueComparer<,>).MakeGenericType(modelType, converter.ProviderClrType)
-                        : modelType == converter.ModelClrType
-                            ? typeof(CurrentProviderValueComparer<,>).MakeGenericType(modelType, converter.ProviderClrType)
-                            : typeof(NullableStructCurrentProviderValueComparer<,>).MakeGenericType(
-                                nonNullableModelType, converter.ProviderClrType);
+                    Type comparerType;
+                    if (QuirkEnabled29985)
+                    {
+                        comparerType = modelType.IsClass
+                            ? typeof(NullableClassCurrentProviderValueComparer<,>).MakeGenericType(modelType, converter.ProviderClrType)
+                            : modelType == converter.ModelClrType
+                                ? typeof(CurrentProviderValueComparer<,>).MakeGenericType(modelType, converter.ProviderClrType)
+                                : typeof(NullableStructCurrentProviderValueComparer<,>).MakeGenericType(
+                                    nonNullableModelType, converter.ProviderClrType);
+                    }
+                    else
+                    {
+                        comparerType = modelType.IsClass
+                            ? typeof(NullableClassCurrentProviderValueComparer<,>).MakeGenericType(modelType, providerType)
+                            : modelType == converter.ModelClrType
+                                ? typeof(CurrentProviderValueComparer<,>).MakeGenericType(modelType, providerType)
+                                : typeof(NullableStructCurrentProviderValueComparer<,>).MakeGenericType(
+                                    nonNullableModelType, providerType);
+                    }
 
                     return (IComparer<IUpdateEntry>)Activator.CreateInstance(comparerType, propertyBase, converter)!;
                 }

src/EFCore/ChangeTracking/ValueComparer`.cs

@@ -117,8 +117,7 @@ public ValueComparer(
             || unwrappedType == typeof(Guid)
             || unwrappedType == typeof(bool)
             || unwrappedType == typeof(decimal)
-            || unwrappedType == typeof(object)
-           )
+            || unwrappedType == typeof(object))
         {
             return Expression.Lambda<Func<T?, T?, bool>>(
                 Expression.Equal(param1, param2),

src/EFCore/Metadata/Internal/Property.cs

@@ -30,6 +30,9 @@ public class Property : PropertyBase, IMutableProperty, IConventionProperty, IPr
     private static readonly bool QuirkEnabled29642
         = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29642", out var enabled) && enabled;
 
+    private static readonly bool QuirkEnabled29985
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29985", out var enabled) && enabled;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -803,8 +806,8 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
         => FindAnnotation(CoreAnnotationNames.ProviderClrType)?.GetConfigurationSource();
 
     private Type GetEffectiveProviderClrType()
-        => TypeMapping?.Converter?.ProviderClrType
-            ?? ClrType.UnwrapNullableType();
+        => (TypeMapping?.Converter?.ProviderClrType
+            ?? ClrType).UnwrapNullableType();
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -1012,10 +1015,20 @@ public virtual CoreTypeMapping? TypeMapping
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual ValueComparer? GetProviderValueComparer()
-        => GetProviderValueComparer(null)
-            ?? (GetEffectiveProviderClrType() == ClrType
+    {
+        if (QuirkEnabled29985)
+        {
+            return GetProviderValueComparer(null)
+                ?? (GetEffectiveProviderClrType() == ClrType
+                    ? GetKeyValueComparer()
+                    : TypeMapping?.ProviderValueComparer);
+        }
+
+        return GetProviderValueComparer(null)
+            ?? (GetEffectiveProviderClrType() == ClrType.UnwrapNullableType()
                 ? GetKeyValueComparer()
                 : TypeMapping?.ProviderValueComparer);
+    }
 
     private ValueComparer? GetProviderValueComparer(HashSet<IProperty>? checkedProperties)
     {

test/EFCore.Relational.Specification.Tests/EFCore.Relational.Specification.Tests.csproj

@@ -53,6 +53,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <ProjectReference Include="..\..\src\EFCore.Analyzers\EFCore.Analyzers.csproj" />
     <ProjectReference Include="..\..\src\EFCore.Design\EFCore.Design.csproj" />
     <ProjectReference Include="..\..\src\EFCore.Relational\EFCore.Relational.csproj" />
     <ProjectReference Include="..\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj" />

test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs

@@ -421,22 +421,26 @@ public virtual void Detects_incompatible_primary_key_columns_with_shared_table()
     }
 
     [ConditionalFact]
-    public virtual void Passes_on_not_configured_shared_columns_with_shared_table()
+    public virtual void Passes_on_shared_columns_with_shared_table()
     {
         var modelBuilder = CreateConventionModelBuilder();
 
         modelBuilder.Entity<A>().HasOne<B>().WithOne(b => b.A).HasForeignKey<A>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();
         modelBuilder.Entity<A>().Property(a => a.P0).HasColumnName(nameof(A.P0));
+        modelBuilder.Entity<A>().Property(a => a.P3).HasColumnName(nameof(A.P3))
+            .HasConversion(e => (long?)e, e => (int?)e);
         modelBuilder.Entity<A>().Property(a => a.P1).IsRequired();
         modelBuilder.Entity<A>().ToTable("Table");
         modelBuilder.Entity<B>().Property(b => b.P0).HasColumnName(nameof(A.P0)).HasColumnType("someInt");
+        modelBuilder.Entity<B>().Property(b => b.P3).HasColumnName(nameof(A.P3))
+            .HasConversion(e => (long)e, e => (int?)e);
         modelBuilder.Entity<B>().ToTable("Table");
 
         Validate(modelBuilder);
     }
 
     [ConditionalFact]
-    public virtual void Throws_on_not_configured_shared_columns_with_shared_table_with_dependents()
+    public virtual void Throws_on_nullable_shared_columns_with_shared_table_with_dependents()
     {
         var modelBuilder = CreateConventionModelBuilder();
 
@@ -450,7 +454,7 @@ public virtual void Throws_on_not_configured_shared_columns_with_shared_table_wi
     }
 
     [ConditionalFact]
-    public virtual void Warns_on_not_configured_shared_columns_with_shared_table()
+    public virtual void Warns_on_no_required_columns_with_shared_table()
     {
         var modelBuilder = CreateConventionModelBuilder();
 

test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs

@@ -10281,6 +10281,36 @@ public void SeedData_key_refactoring_value_conversion()
             Assert.Empty,
             Assert.Empty);
 
+    [ConditionalFact] // Issue #29985
+    public void SeedData_value_conversion_nullable_datetime()
+        => Execute(
+            common => common.Entity(
+                "EntityWithOneProperty",
+                x =>
+                {
+                    x.Property<int>("Id");
+                    x.HasData(new { Id = 42 });
+                }),
+            source => source.Entity(
+                "EntityWithOneProperty",
+                x =>
+                {
+                    x.Property<DateTime?>("Value1")
+                        .HasColumnType("datetime2")
+                        .HasConversion(
+                            p => p,
+                            p => p != null ? DateTime.SpecifyKind(p.Value, DateTimeKind.Utc) : null);
+                }),
+            target => target.Entity(
+                "EntityWithOneProperty",
+                x =>
+                {
+                    x.Property<DateTime?>("Value1")
+                        .HasColumnType("datetime2");
+                }),
+            Assert.Empty,
+            Assert.Empty);
+
     [ConditionalFact]
     public void SeedData_change_enum_conversion()
         => Execute(