Skip to content

Commit

Permalink
Query: Add query logger to member/method translators (#21935)
Browse files Browse the repository at this point in the history
Resolves #17498
  • Loading branch information
smitpatel committed Aug 5, 2020
1 parent 67c3dc1 commit 8f7b508
Show file tree
Hide file tree
Showing 74 changed files with 409 additions and 117 deletions.
4 changes: 3 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/ContainsTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand Down Expand Up @@ -34,7 +35,8 @@ public ContainsTranslator([NotNull] ISqlExpressionFactory sqlExpressionFactory)
/// 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 virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
public virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments,
IDiagnosticsLogger<DbLoggerCategory.Query> logger)
{
if (method.IsGenericMethod
&& method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
Expand Down Expand Up @@ -46,14 +47,16 @@ public CosmosMemberTranslatorProvider(
/// 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 virtual SqlExpression Translate(SqlExpression instance, MemberInfo member, Type returnType)
public virtual SqlExpression Translate(
SqlExpression instance, MemberInfo member, Type returnType, IDiagnosticsLogger<DbLoggerCategory.Query> logger)
{
Check.NotNull(instance, nameof(instance));
Check.NotNull(member, nameof(member));
Check.NotNull(returnType, nameof(returnType));
Check.NotNull(logger, nameof(logger));

return _plugins.Concat(_translators)
.Select(t => t.Translate(instance, member, returnType)).FirstOrDefault(t => t != null);
.Select(t => t.Translate(instance, member, returnType, logger)).FirstOrDefault(t => t != null);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Utilities;

Expand Down Expand Up @@ -53,14 +54,16 @@ public CosmosMethodCallTranslatorProvider(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual SqlExpression Translate(
IModel model, SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
IModel model, SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments,
IDiagnosticsLogger<DbLoggerCategory.Query> logger)
{
Check.NotNull(model, nameof(model));
Check.NotNull(method, nameof(method));
Check.NotNull(arguments, nameof(arguments));
Check.NotNull(logger, nameof(logger));

return _plugins.Concat(_translators)
.Select(t => t.Translate(instance, method, arguments))
.Select(t => t.Translate(instance, method, arguments, logger))
.FirstOrDefault(t => t != null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ protected override Expression VisitMember(MemberExpression memberExpression)
return TryBindMember(innerExpression, MemberIdentity.Create(memberExpression.Member))
?? (TranslationFailed(memberExpression.Expression, innerExpression, out var sqlInnerExpression)
? null
: _memberTranslatorProvider.Translate(sqlInnerExpression, memberExpression.Member, memberExpression.Type));
: _memberTranslatorProvider.Translate(
sqlInnerExpression, memberExpression.Member, memberExpression.Type, _queryCompilationContext.Logger));
}

/// <summary>
Expand Down Expand Up @@ -490,7 +491,8 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
}
}

var translation = _methodCallTranslatorProvider.Translate(_model, sqlObject, methodCallExpression.Method, arguments);
var translation = _methodCallTranslatorProvider.Translate(
_model, sqlObject, methodCallExpression.Method, arguments, _queryCompilationContext.Logger);

if (translation == null)
{
Expand Down
5 changes: 4 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/EqualsTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq.Expressions;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
Expand Down Expand Up @@ -37,10 +38,12 @@ public EqualsTranslator([NotNull] ISqlExpressionFactory sqlExpressionFactory)
/// 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 virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
public virtual SqlExpression Translate(
SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments, IDiagnosticsLogger<DbLoggerCategory.Query> logger)
{
Check.NotNull(method, nameof(method));
Check.NotNull(arguments, nameof(arguments));
Check.NotNull(logger, nameof(logger));

SqlExpression left = null;
SqlExpression right = null;
Expand Down
4 changes: 3 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/IMemberTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -21,6 +22,7 @@ public interface IMemberTranslator
/// 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>
SqlExpression Translate([NotNull] SqlExpression instance, [NotNull] MemberInfo member, [NotNull] Type returnType);
SqlExpression Translate([NotNull] SqlExpression instance, [NotNull] MemberInfo member, [NotNull] Type returnType,
[NotNull] IDiagnosticsLogger<DbLoggerCategory.Query> logger);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -21,6 +22,7 @@ public interface IMemberTranslatorProvider
/// 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>
SqlExpression Translate([NotNull] SqlExpression instance, [NotNull] MemberInfo member, [NotNull] Type returnType);
SqlExpression Translate([NotNull] SqlExpression instance, [NotNull] MemberInfo member, [NotNull] Type returnType,
[NotNull] IDiagnosticsLogger<DbLoggerCategory.Query> logger);
}
}
4 changes: 3 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/IMethodCallTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -22,6 +23,7 @@ public interface IMethodCallTranslator
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
SqlExpression Translate(
[CanBeNull] SqlExpression instance, [NotNull] MethodInfo method, [NotNull] IReadOnlyList<SqlExpression> arguments);
[CanBeNull] SqlExpression instance, [NotNull] MethodInfo method, [NotNull] IReadOnlyList<SqlExpression> arguments,
[NotNull] IDiagnosticsLogger<DbLoggerCategory.Query> logger);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
Expand All @@ -26,6 +27,7 @@ SqlExpression Translate(
[NotNull] IModel model,
[CanBeNull] SqlExpression instance,
[NotNull] MethodInfo method,
[NotNull] IReadOnlyList<SqlExpression> arguments);
[NotNull] IReadOnlyList<SqlExpression> arguments,
[NotNull] IDiagnosticsLogger<DbLoggerCategory.Query> logger);
}
}
5 changes: 4 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
Expand Down Expand Up @@ -56,10 +57,12 @@ public StringMethodTranslator([NotNull] ISqlExpressionFactory sqlExpressionFacto
/// 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 virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
public virtual SqlExpression Translate(
SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments, IDiagnosticsLogger<DbLoggerCategory.Query> logger)
{
Check.NotNull(method, nameof(method));
Check.NotNull(arguments, nameof(arguments));
Check.NotNull(logger, nameof(logger));

if (_containsMethodInfo.Equals(method))
{
Expand Down
22 changes: 14 additions & 8 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
using System.Transactions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Migrations.Internal;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.Internal;
using Microsoft.EntityFrameworkCore.Update;
Expand Down Expand Up @@ -4129,34 +4132,37 @@ private static string MigrationAttributeMissingWarning(EventDefinitionBase defin
/// Logs for the <see cref="RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="methodCallExpression"> The expression representing the problematic method call. </param>
/// <param name="left"> The left SQL expression of the Equals. </param>
/// <param name="right"> The right SQL expression of the Equals. </param>
public static void QueryPossibleUnintendedUseOfEqualsWarning(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
[NotNull] MethodCallExpression methodCallExpression)
[NotNull] SqlExpression left,
[NotNull] SqlExpression right)
{
var definition = RelationalResources.LogPossibleUnintendedUseOfEquals(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, methodCallExpression);
definition.Log(diagnostics, left.Print(), right.Print());
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new ExpressionEventData(
var eventData = new TwoSqlExpressionEventData(
definition,
QueryPossibleUnintendedUseOfEqualsWarning,
methodCallExpression);
left,
right);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
}
}

private static string QueryPossibleUnintendedUseOfEqualsWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<object>)definition;
var p = (ExpressionEventData)payload;
return d.GenerateMessage(p.Expression);
var d = (EventDefinition<string, string>)definition;
var p = (TwoSqlExpressionEventData)payload;
return d.GenerateMessage(p.Left.Print(), p.Right.Print());
}

/// <summary>
Expand Down
45 changes: 45 additions & 0 deletions src/EFCore.Relational/Diagnostics/TwoSqlExpressionEventData.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Diagnostics;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;

namespace Microsoft.EntityFrameworkCore.Diagnostics
{
/// <summary>
/// The <see cref="DiagnosticSource" /> event payload base class for events that
/// references two <see cref="SqlExpression"/>.
/// </summary>
public class TwoSqlExpressionEventData : EventData
{
/// <summary>
/// Constructs the event payload.
/// </summary>
/// <param name="eventDefinition"> The event definition. </param>
/// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
/// <param name="left"> The left SqlExpression. </param>
/// <param name="right"> The right SqlExpression. </param>
public TwoSqlExpressionEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] SqlExpression left,
[NotNull] SqlExpression right)
: base(eventDefinition, messageGenerator)
{
Left = left;
Right = right;
}

/// <summary>
/// The left SqlExpression.
/// </summary>
public virtual SqlExpression Left { get; }

/// <summary>
/// The right SqlExpression.
/// </summary>
public virtual SqlExpression Right { get; }
}
}
10 changes: 5 additions & 5 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

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

4 changes: 2 additions & 2 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@
<comment>Warning RelationalEventId.AmbientTransactionWarning</comment>
</data>
<data name="LogPossibleUnintendedUseOfEquals" xml:space="preserve">
<value>Possible unintended use of method Equals(object) for arguments of different types in expression '{expression}'. This comparison will always return 'false'.</value>
<comment>Warning RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning object</comment>
<value>Possible unintended use of method Equals(object) for arguments '{left}' and '{right}' of different types in query. This comparison will always return 'false'.</value>
<comment>Warning RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning string string</comment>
</data>
<data name="LogQueryPossibleExceptionWithAggregateOperatorWarning" xml:space="preserve">
<value>Possible unintended use of a potentially throwing aggregate method (Min, Max, Average) in a subquery. Client evaluation will be used and operator will throw if no data exists. Changing the subquery result type to a nullable type will allow full translation.</value>
Expand Down
8 changes: 7 additions & 1 deletion src/EFCore.Relational/Query/IMemberTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq.Expressions;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;

namespace Microsoft.EntityFrameworkCore.Query
Expand All @@ -26,7 +27,12 @@ public interface IMemberTranslator
/// <param name="instance"> A SQL representation of <see cref="MemberExpression.Expression"/>. </param>
/// <param name="member"> The member info from <see cref="MemberExpression.Member"/>. </param>
/// <param name="returnType"> The return type from <see cref="P:MemberExpression.Type"/>. </param>
/// <param name="logger"> The query logger to use. </param>
/// <returns> A SQL translation of the <see cref="MemberExpression"/>. </returns>
SqlExpression Translate([CanBeNull] SqlExpression instance, [NotNull] MemberInfo member, [NotNull] Type returnType);
SqlExpression Translate(
[CanBeNull] SqlExpression instance,
[NotNull] MemberInfo member,
[NotNull] Type returnType,
[NotNull] IDiagnosticsLogger<DbLoggerCategory.Query> logger);
}
}
Loading

0 comments on commit 8f7b508

Please sign in to comment.