Skip to content

Commit

Permalink
Query: Generate a warning for multiple collection Includes
Browse files Browse the repository at this point in the history
Resolves #19933
  • Loading branch information
smitpatel committed Jun 26, 2020
1 parent 04e605b commit b0db86c
Show file tree
Hide file tree
Showing 10 changed files with 276 additions and 36 deletions.
11 changes: 11 additions & 0 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ private enum Id
QueryPossibleUnintendedUseOfEqualsWarning,
QueryPossibleExceptionWithAggregateOperatorWarning,
ValueConversionSqlLiteralWarning, // This warning has been removed.
MultipleCollectionIncludeWarning,

// Model validation events
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
Expand Down Expand Up @@ -611,6 +612,16 @@ private enum Id
public static readonly EventId QueryPossibleExceptionWithAggregateOperatorWarning =
MakeQueryId(Id.QueryPossibleExceptionWithAggregateOperatorWarning);

/// <summary>
/// <para>
/// A query is loading multiple related collections without configuring a <see cref="QuerySplittingBehavior"/>.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
/// </para>
/// </summary>
public static readonly EventId MultipleCollectionIncludeWarning = MakeQueryId(Id.MultipleCollectionIncludeWarning);

private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + ".";
private static EventId MakeValidationId(Id id) => new EventId((int)id, _validationPrefix + id);

Expand Down
24 changes: 24 additions & 0 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4183,6 +4183,30 @@ public static void QueryPossibleExceptionWithAggregateOperatorWarning(
}
}

/// <summary>
/// Logs for the <see cref="RelationalEventId.MultipleCollectionIncludeWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
public static void MultipleCollectionIncludeWarning(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics)
{
var definition = RelationalResources.LogMultipleCollectionIncludeWarning(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new EventData(
definition,
(d, p) => ((EventDefinition)d).GenerateMessage());

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

/// <summary>
/// Logs for the <see cref="RelationalEventId.ModelValidationKeyDefaultValueWarning" /> event.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,5 +474,14 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
/// </summary>
[EntityFrameworkInternal]
public EventDefinitionBase LogUnnamedIndexPropertiesMappedToNonOverlappingTables;

/// <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>
[EntityFrameworkInternal]
public EventDefinitionBase LogMultipleCollectionIncludeWarning;
}
}
24 changes: 24 additions & 0 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: 4 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -660,4 +660,8 @@
<data name="IncompatibleTableExcludedMismatch" xml:space="preserve">
<value>Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and it excluded from migration on one entity type, but not the other. Exclude the table from migrations on all entity types mapped to the table.</value>
</data>
<data name="LogMultipleCollectionIncludeWarning" xml:space="preserve">
<value>Compiling a query which loads related collections for more than one collection navigation property either via 'Include' or through projection but no 'QuerySplittingBehavior' has been configured. By default EF Core will use 'QuerySplittingBehavior.SingleQuery' which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call .ConfigureWarnings(w =&gt; w.Throw(RelationalEventId.LogTooManyIncludesWarning))</value>
<comment>Warning RelationalEventId.MultipleCollectionIncludeWarning</comment>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Utilities;
Expand All @@ -18,7 +19,8 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal
public class CollectionJoinApplyingExpressionVisitor : ExpressionVisitor
{
private readonly bool _splitQuery;
private readonly bool _userConfiguredBehavior;
private readonly bool _noConfiguredBehavior;
private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _logger;
private int _collectionId;

/// <summary>
Expand All @@ -32,7 +34,8 @@ public CollectionJoinApplyingExpressionVisitor([NotNull] RelationalQueryCompilat
Check.NotNull(queryCompilationContext, nameof(queryCompilationContext));

_splitQuery = queryCompilationContext.QuerySplittingBehavior == QuerySplittingBehavior.SplitQuery;
_userConfiguredBehavior = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).QuerySplittingBehavior.HasValue;
_noConfiguredBehavior = queryCompilationContext.QuerySplittingBehavior == null;
_logger = queryCompilationContext.Logger;
}

/// <summary>
Expand All @@ -48,6 +51,13 @@ protected override Expression VisitExtension(Expression extensionExpression)
if (extensionExpression is CollectionShaperExpression collectionShaperExpression)
{
var collectionId = _collectionId++;

if (_noConfiguredBehavior
&& _collectionId == 2)
{
_logger.MultipleCollectionIncludeWarning();
}

var projectionBindingExpression = (ProjectionBindingExpression)collectionShaperExpression.Projection;
var selectExpression = (SelectExpression)projectionBindingExpression.QueryExpression;
// Do pushdown beforehand so it updates all pending collections first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ public RelationalQueryCompilationContext(
Check.NotNull(relationalDependencies, nameof(relationalDependencies));

RelationalDependencies = relationalDependencies;
QuerySplittingBehavior = RelationalOptionsExtension.Extract(ContextOptions).QuerySplittingBehavior
?? QuerySplittingBehavior.SingleQuery;
QuerySplittingBehavior = RelationalOptionsExtension.Extract(ContextOptions).QuerySplittingBehavior;
}

/// <summary>
Expand All @@ -43,8 +42,9 @@ public RelationalQueryCompilationContext(
protected virtual RelationalQueryCompilationContextDependencies RelationalDependencies { get; }

/// <summary>
/// A value indicating the <see cref="EntityFrameworkCore.QuerySplittingBehavior"/> of the query.
/// A value indicating the <see cref="EntityFrameworkCore.QuerySplittingBehavior"/> configured for the query.
/// If no value has been configured then <see cref="QuerySplittingBehavior.SingleQuery"/> will be used.
/// </summary>
public virtual QuerySplittingBehavior QuerySplittingBehavior { get; internal set; }
public virtual QuerySplittingBehavior? QuerySplittingBehavior { get; internal set; }
}
}
Loading

0 comments on commit b0db86c

Please sign in to comment.