Skip to content

Commit

Permalink
Reimplement entity equality
Browse files Browse the repository at this point in the history
New implementation of entity equality rewriter, which follows exact
entity (and anonymous) from roots to the comparison expression, for
more precise rewriting.

Closes #15588
  • Loading branch information
roji committed Jun 6, 2019
1 parent 5df2582 commit 6dbf690
Show file tree
Hide file tree
Showing 18 changed files with 976 additions and 120 deletions.
2 changes: 1 addition & 1 deletion src/EFCore/Diagnostics/CoreEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ private enum Id
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationPathEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId PossibleUnintendedCollectionNavigationNullComparisonWarning
Expand Down
14 changes: 7 additions & 7 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -503,10 +503,10 @@ public static void SensitiveDataLoggingEnabledWarning<TLoggerCategory>(
/// Logs for the <see cref="CoreEventId.PossibleUnintendedCollectionNavigationNullComparisonWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigationPath"> The navigation properties being used. </param>
/// <param name="navigation"> The navigation being used. </param>
public static void PossibleUnintendedCollectionNavigationNullComparisonWarning(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
[NotNull] IReadOnlyList<IPropertyBase> navigationPath)
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogPossibleUnintendedCollectionNavigationNullComparison(diagnostics);

Expand All @@ -516,25 +516,25 @@ public static void PossibleUnintendedCollectionNavigationNullComparisonWarning(
definition.Log(
diagnostics,
warningBehavior,
string.Join(".", navigationPath.Select(p => p.Name)));
$"{navigation.DeclaringEntityType.Name}.{navigation.GetTargetType().Name}");
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new NavigationPathEventData(
new NavigationEventData(
definition,
PossibleUnintendedCollectionNavigationNullComparisonWarning,
navigationPath));
navigation));
}
}

private static string PossibleUnintendedCollectionNavigationNullComparisonWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string>)definition;
var p = (NavigationPathEventData)payload;
return d.GenerateMessage(string.Join(".", p.NavigationPath.Select(pb => pb.Name)));
var p = (NavigationEventData)payload;
return d.GenerateMessage($"{p.Navigation.DeclaringEntityType.Name}.{p.Navigation.GetTargetType().Name}");
}

/// <summary>
Expand Down
38 changes: 0 additions & 38 deletions src/EFCore/Diagnostics/NavigationPathEventData.cs

This file was deleted.

29 changes: 0 additions & 29 deletions src/EFCore/Extensions/Internal/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -373,35 +373,6 @@ public static bool IsNullPropagationCandidate(
return true;
}

/// <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 Expression CreateKeyAccessExpression(
[NotNull] this Expression target,
[NotNull] IReadOnlyList<IProperty> properties)
{
Check.NotNull(target, nameof(target));
Check.NotNull(properties, nameof(properties));

return properties.Count == 1
? target.CreateEFPropertyExpression(properties[0])
: Expression.New(
AnonymousObject.AnonymousObjectCtor,
Expression.NewArrayInit(
typeof(object),
properties
.Select(
p =>
Expression.Convert(
target.CreateEFPropertyExpression(p),
typeof(object)))
.Cast<Expression>()
.ToArray()));
}

/// <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
8 changes: 8 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1168,4 +1168,7 @@
<data name="NoNavigation" xml:space="preserve">
<value>There is no navigation on entity type '{entityType}' associated with the foreign key {foreignKey}.</value>
</data>
</root>
<data name="SubqueryWithCompositeKeyNotSupported" xml:space="preserve">
<value>This query would cause multiple evaluation of a subquery because entity '{entityType}' has a composite key. Rewrite your query avoiding the subquery.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,13 @@ var operation
/// </returns>
/// <param name="visitor">An instance of <see cref="T:System.Func`2" />.</param>
protected override Expression VisitChildren(ExpressionVisitor visitor)
{
var newCaller = visitor.Visit(Caller);
var newAccessOperation = visitor.Visit(AccessOperation);
=> Update(visitor.Visit(Caller), visitor.Visit(AccessOperation));

return newCaller != Caller
|| newAccessOperation != AccessOperation
&& !(ExpressionEqualityComparer.Instance.Equals((newAccessOperation as NullConditionalExpression)?.AccessOperation, AccessOperation))
public virtual Expression Update(Expression newCaller, Expression newAccessOperation)
=> newCaller != Caller || newAccessOperation != AccessOperation
&& !ExpressionEqualityComparer.Instance.Equals((newAccessOperation as NullConditionalExpression)?.AccessOperation, AccessOperation)
? new NullConditionalExpression(newCaller, newAccessOperation)
: (this);
}
: this;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Loading

0 comments on commit 6dbf690

Please sign in to comment.