Use correct value comparer for FKs in compiled model.

Fixes #28108

src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs

@@ -0,0 +1,73 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
+
+/// <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
+/// any release. You should only use it directly in your code with extreme caution and knowing that
+/// doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public static class ValueComparerExtensions
+{
+ /// <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
+ /// any release. You should only use it directly in your code with extreme caution and knowing that
+ /// doing so can result in application failures when updating to a new Entity Framework Core release.
+ /// </summary>
+ public static ValueComparer? ToNullableComparer(this ValueComparer? valueComparer, IReadOnlyProperty property)
+ {
+ if (valueComparer == null
+ || !property.ClrType.IsNullableValueType()
+ || valueComparer.Type.IsNullableValueType())
+ {
+ return valueComparer;
+ }
+
+ var newEqualsParam1 = Expression.Parameter(property.ClrType, "v1");
+ var newEqualsParam2 = Expression.Parameter(property.ClrType, "v2");
+ var newHashCodeParam = Expression.Parameter(property.ClrType, "v");
+ var newSnapshotParam = Expression.Parameter(property.ClrType, "v");
+ var hasValueMethod = property.ClrType.GetMethod("get_HasValue")!;
+ var v1HasValue = Expression.Parameter(typeof(bool), "v1HasValue");
+ var v2HasValue = Expression.Parameter(typeof(bool), "v2HasValue");
+
+ return (ValueComparer)Activator.CreateInstance(
+ typeof(ValueComparer<>).MakeGenericType(property.ClrType),
+ Expression.Lambda(
+ Expression.Block(
+ typeof(bool),
+ new[] { v1HasValue, v2HasValue },
+ Expression.Assign(v1HasValue, Expression.Call(newEqualsParam1, hasValueMethod)),
+ Expression.Assign(v2HasValue, Expression.Call(newEqualsParam2, hasValueMethod)),
+ Expression.OrElse(
+ Expression.AndAlso(
+ v1HasValue,
+ Expression.AndAlso(
+ v2HasValue,
+ valueComparer.ExtractEqualsBody(
+ Expression.Convert(newEqualsParam1, valueComparer.Type),
+ Expression.Convert(newEqualsParam2, valueComparer.Type)))),
+ Expression.AndAlso(
+ Expression.Not(v1HasValue),
+ Expression.Not(v2HasValue)))),
+ newEqualsParam1, newEqualsParam2),
+ Expression.Lambda(
+ Expression.Condition(
+ Expression.Call(newHashCodeParam, hasValueMethod),
+ valueComparer.ExtractHashCodeBody(
+ Expression.Convert(newHashCodeParam, valueComparer.Type)),
+ Expression.Constant(0, typeof(int))),
+ newHashCodeParam),
+ Expression.Lambda(
+ Expression.Condition(
+ Expression.Call(newSnapshotParam, hasValueMethod),
+ Expression.Convert(
+ valueComparer.ExtractSnapshotBody(
+ Expression.Convert(newSnapshotParam, valueComparer.Type)), property.ClrType),
+ Expression.Default(property.ClrType)),
+ newSnapshotParam))!;
+ }
+}

src/EFCore/Metadata/Internal/Property.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics.CodeAnalysis;
+using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
@@ -815,8 +816,7 @@ public virtual CoreTypeMapping? TypeMapping
  /// doing so can result in application failures when updating to a new Entity Framework Core release.
  /// </summary>
  public virtual ValueComparer? GetValueComparer()
- => ToNullableComparer(GetValueComparer(null)
- ?? TypeMapping?.Comparer);
+ => (GetValueComparer(null) ?? TypeMapping?.Comparer).ToNullableComparer(this);
 
  private ValueComparer? GetValueComparer(HashSet<IProperty>? checkedProperties)
  {
@@ -826,7 +826,7 @@ public virtual CoreTypeMapping? TypeMapping
  return comparer;
  }
 
- var principal = (Property?)FindFirstDifferentPrincipal();
+ var principal = (Property?)this.FindFirstDifferentPrincipal();
  if (principal == null)
  {
  return null;
@@ -861,8 +861,7 @@ public virtual CoreTypeMapping? TypeMapping
  /// doing so can result in application failures when updating to a new Entity Framework Core release.
  /// </summary>
  public virtual ValueComparer? GetKeyValueComparer()
- => ToNullableComparer(GetValueComparer(null)
- ?? TypeMapping?.KeyComparer);
+ => (GetValueComparer(null) ?? TypeMapping?.KeyComparer).ToNullableComparer(this);
 
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -928,7 +927,7 @@ public virtual CoreTypeMapping? TypeMapping
  return comparer;
  }
 
- var principal = (Property?)FindFirstDifferentPrincipal();
+ var principal = (Property?)this.FindFirstDifferentPrincipal();
  if (principal == null
  || principal.GetEffectiveProviderClrType() != GetEffectiveProviderClrType())
  {
@@ -947,7 +946,7 @@ public virtual CoreTypeMapping? TypeMapping
  checkedProperties.Add(this);
  return principal.GetProviderValueComparer(checkedProperties);
  }
- 
+
  /// <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
@@ -957,67 +956,6 @@ public virtual CoreTypeMapping? TypeMapping
  public virtual ConfigurationSource? GetProviderValueComparerConfigurationSource()
  => FindAnnotation(CoreAnnotationNames.ProviderValueComparer)?.GetConfigurationSource();
 
- private ValueComparer? ToNullableComparer(ValueComparer? valueComparer)
- {
- if (valueComparer == null
- || !ClrType.IsNullableValueType()
- || valueComparer.Type.IsNullableValueType())
- {
- return valueComparer;
- }
-
- var newEqualsParam1 = Expression.Parameter(ClrType, "v1");
- var newEqualsParam2 = Expression.Parameter(ClrType, "v2");
- var newHashCodeParam = Expression.Parameter(ClrType, "v");
- var newSnapshotParam = Expression.Parameter(ClrType, "v");
- var hasValueMethod = ClrType.GetMethod("get_HasValue")!;
- var v1HasValue = Expression.Parameter(typeof(bool), "v1HasValue");
- var v2HasValue = Expression.Parameter(typeof(bool), "v2HasValue");
-
- return (ValueComparer)Activator.CreateInstance(
- typeof(ValueComparer<>).MakeGenericType(ClrType),
- Expression.Lambda(
- Expression.Block(
- typeof(bool),
- new[] { v1HasValue, v2HasValue },
- Expression.Assign(v1HasValue, Expression.Call(newEqualsParam1, hasValueMethod)),
- Expression.Assign(v2HasValue, Expression.Call(newEqualsParam2, hasValueMethod)),
- Expression.OrElse(
- Expression.AndAlso(
- v1HasValue,
- Expression.AndAlso(
- v2HasValue,
- valueComparer.ExtractEqualsBody(
- Expression.Convert(newEqualsParam1, valueComparer.Type),
- Expression.Convert(newEqualsParam2, valueComparer.Type)))),
- Expression.AndAlso(
- Expression.Not(v1HasValue),
- Expression.Not(v2HasValue)))),
- newEqualsParam1, newEqualsParam2),
- Expression.Lambda(
- Expression.Condition(
- Expression.Call(newHashCodeParam, hasValueMethod),
- valueComparer.ExtractHashCodeBody(
- Expression.Convert(newHashCodeParam, valueComparer.Type)),
- Expression.Constant(0, typeof(int))),
- newHashCodeParam),
- Expression.Lambda(
- Expression.Condition(
- Expression.Call(newSnapshotParam, hasValueMethod),
- Expression.Convert(
- valueComparer.ExtractSnapshotBody(
- Expression.Convert(newSnapshotParam, valueComparer.Type)), ClrType),
- Expression.Default(ClrType)),
- newSnapshotParam))!;
- }
-
- private IProperty? FindFirstDifferentPrincipal()
- {
- var principal = ((IProperty)this).FindFirstPrincipal();
-
- return principal != this ? principal : null;
- }
-
  /// <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

src/EFCore/Metadata/Internal/PropertyExtensions.cs

@@ -1,6 +1,8 @@
 // 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.ChangeTracking.Internal;
+
 namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
 
 /// <summary>
@@ -29,6 +31,19 @@ public static bool ForAdd(this ValueGenerated valueGenerated)
  public static bool ForUpdate(this ValueGenerated valueGenerated)
  => (valueGenerated & ValueGenerated.OnUpdate) != 0;
 
+ /// <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
+ /// any release. You should only use it directly in your code with extreme caution and knowing that
+ /// doing so can result in application failures when updating to a new Entity Framework Core release.
+ /// </summary>
+ public static IReadOnlyProperty? FindFirstDifferentPrincipal(this IReadOnlyProperty property)
+ {
+ var principal = property.FindFirstPrincipal();
+
+ return principal != property ? principal : null;
+ }
+
  /// <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

src/EFCore/Metadata/RuntimeProperty.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 Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 
@@ -21,8 +22,10 @@ public class RuntimeProperty : RuntimePropertyBase, IProperty
  private readonly PropertySaveBehavior _afterSaveBehavior;
  private readonly Func<IProperty, IEntityType, ValueGenerator>? _valueGeneratorFactory;
  private readonly ValueConverter? _valueConverter;
- private readonly ValueComparer? _valueComparer;
- private readonly ValueComparer? _keyValueComparer;
+ private readonly bool _explicitValueComparer;
+ private ValueComparer? _valueComparer;
+ private readonly bool _explicitKeyValueComparer;
+ private ValueComparer? _keyValueComparer;
  private readonly ValueComparer? _providerValueComparer;
  private CoreTypeMapping? _typeMapping;
 
@@ -95,7 +98,9 @@ public RuntimeProperty(
 
  _typeMapping = typeMapping;
  _valueComparer = valueComparer;
+ _explicitValueComparer = _valueComparer != null;
  _keyValueComparer = keyValueComparer ?? valueComparer;
+ _explicitKeyValueComparer = keyValueComparer != null;
  _providerValueComparer = providerValueComparer;
  }
 
@@ -167,6 +172,68 @@ public virtual CoreTypeMapping TypeMapping
  set => _typeMapping = value;
  }
 
+ private ValueComparer GetValueComparer()
+ => (GetValueComparer(null) ?? TypeMapping.Comparer)
+ .ToNullableComparer(this)!;
+
+ private ValueComparer GetKeyValueComparer()
+ => (GetKeyValueComparer(null) ?? TypeMapping.KeyComparer)
+ .ToNullableComparer(this)!;
+
+ private ValueComparer? GetValueComparer(HashSet<IReadOnlyProperty>? checkedProperties)
+ {
+ if (_explicitValueComparer // This condition is needed due to #28944
+ && _valueComparer != null)
+ {
+ return _valueComparer;
+ }
+
+ var principal = (RuntimeProperty?)this.FindFirstDifferentPrincipal();
+ if (principal == null)
+ {
+ return null;
+ }
+
+ if (checkedProperties == null)
+ {
+ checkedProperties = new HashSet<IReadOnlyProperty>();
+ }
+ else if (checkedProperties.Contains(this))
+ {
+ return null;
+ }
+
+ checkedProperties.Add(this);
+ return principal.GetValueComparer(checkedProperties);
+ }
+
+ private ValueComparer? GetKeyValueComparer(HashSet<IReadOnlyProperty>? checkedProperties)
+ {
+ if (_explicitKeyValueComparer // This condition is needed due to #28944
+ && _keyValueComparer != null)
+ {
+ return _keyValueComparer;
+ }
+
+ var principal = (RuntimeProperty?)this.FindFirstDifferentPrincipal();
+ if (principal == null)
+ {
+ return null;
+ }
+
+ if (checkedProperties == null)
+ {
+ checkedProperties = new HashSet<IReadOnlyProperty>();
+ }
+ else if (checkedProperties.Contains(this))
+ {
+ return null;
+ }
+
+ checkedProperties.Add(this);
+ return principal.GetKeyValueComparer(checkedProperties);
+ }
+
  /// <summary>
  /// Returns a string that represents the current object.
  /// </summary>
@@ -274,22 +341,30 @@ IEntityType IProperty.DeclaringEntityType
  /// <inheritdoc />
  [DebuggerStepThrough]
  ValueComparer? IReadOnlyProperty.GetValueComparer()
- => _valueComparer ?? TypeMapping.Comparer;
+ => NonCapturingLazyInitializer.EnsureInitialized(
+ ref _valueComparer, this,
+ static property => property.GetValueComparer());
 
  /// <inheritdoc />
  [DebuggerStepThrough]
  ValueComparer IProperty.GetValueComparer()
- => _valueComparer ?? TypeMapping.Comparer;
+ => NonCapturingLazyInitializer.EnsureInitialized(
+ ref _valueComparer, this,
+ static property => property.GetValueComparer());
 
  /// <inheritdoc />
  [DebuggerStepThrough]
  ValueComparer? IReadOnlyProperty.GetKeyValueComparer()
- => _keyValueComparer ?? TypeMapping.KeyComparer;
+ => NonCapturingLazyInitializer.EnsureInitialized(
+ ref _keyValueComparer, this,
+ static property => property.GetKeyValueComparer());
 
  /// <inheritdoc />
  [DebuggerStepThrough]
  ValueComparer IProperty.GetKeyValueComparer()
- => _keyValueComparer ?? TypeMapping.KeyComparer;
+ => NonCapturingLazyInitializer.EnsureInitialized(
+ ref _keyValueComparer, this,
+ static property => property.GetKeyValueComparer());
 
  /// <inheritdoc />
  [DebuggerStepThrough]

src/EFCore/Query/ExpressionPrinter.cs

@@ -4,7 +4,6 @@
 using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
-using System;
 
 namespace Microsoft.EntityFrameworkCore.Query;
 
@@ -613,15 +612,17 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
  {
  if (methodCallExpression.Object != null)
  {
- if (methodCallExpression.Object is BinaryExpression)
+ switch (methodCallExpression.Object)
  {
- _stringBuilder.Append("(");
- Visit(methodCallExpression.Object);
- _stringBuilder.Append(")");
- }
- else
- {
- Visit(methodCallExpression.Object);
+ case BinaryExpression:
+ case UnaryExpression:
+ _stringBuilder.Append("(");
+ Visit(methodCallExpression.Object);
+ _stringBuilder.Append(")");
+ break;
+ default:
+ Visit(methodCallExpression.Object);
+ break;
  }
 
  _stringBuilder.Append(".");