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

Query: Generate a warning for multiple collection Includes #21419

Merged
merged 1 commit into from
Jun 26, 2020
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
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