Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct value comparer for FKs in compiled model. #28950

Merged
merged 2 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs
Original file line number Diff line number Diff line change
@@ -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))!;
}
}
74 changes: 6 additions & 68 deletions src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -826,7 +826,7 @@ public virtual CoreTypeMapping? TypeMapping
return comparer;
}

var principal = (Property?)FindFirstDifferentPrincipal();
var principal = (Property?)this.FindFirstDifferentPrincipal();
if (principal == null)
{
return null;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
{
Expand All @@ -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
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/EFCore/Metadata/Internal/PropertyExtensions.cs
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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
Expand Down
87 changes: 81 additions & 6 deletions src/EFCore/Metadata/RuntimeProperty.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -95,7 +98,9 @@ public RuntimeProperty(

_typeMapping = typeMapping;
_valueComparer = valueComparer;
_explicitValueComparer = _valueComparer != null;
_keyValueComparer = keyValueComparer ?? valueComparer;
_explicitKeyValueComparer = keyValueComparer != null;
_providerValueComparer = providerValueComparer;
}

Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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]
Expand Down
19 changes: 10 additions & 9 deletions src/EFCore/Query/ExpressionPrinter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Collections;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System;

namespace Microsoft.EntityFrameworkCore.Query;

Expand Down Expand Up @@ -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(".");
Expand Down
Loading