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.

Sign up for GitHub

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

Jump to bottom

API review changes #21960

Merged
1 commit merged into from
Aug 6, 2020
Merged

API review changes #21960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Diagnostics/RelationalEventId.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ private enum Id
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="ExpressionEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// This event uses the <see cref="TwoSqlExpressionsEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId QueryPossibleUnintendedUseOfEqualsWarning =
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4148,7 +4148,7 @@ public static void QueryPossibleUnintendedUseOfEqualsWarning(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new TwoSqlExpressionEventData(
var eventData = new TwoSqlExpressionsEventData(
definition,
QueryPossibleUnintendedUseOfEqualsWarning,
left,
Expand All @@ -4161,7 +4161,7 @@ public static void QueryPossibleUnintendedUseOfEqualsWarning(
private static string QueryPossibleUnintendedUseOfEqualsWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (TwoSqlExpressionEventData)payload;
var p = (TwoSqlExpressionsEventData)payload;
return d.GenerateMessage(p.Left.Print(), p.Right.Print());
}

Expand Down
4 changes: 2 additions & 2 deletions .../Diagnostics/TwoSqlExpressionEventData.cs → ...Diagnostics/TwoSqlExpressionsEventData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
/// The <see cref="DiagnosticSource" /> event payload base class for events that
/// references two <see cref="SqlExpression"/>.
/// </summary>
public class TwoSqlExpressionEventData : EventData
public class TwoSqlExpressionsEventData : EventData
{
/// <summary>
/// Constructs the event payload.
Expand All @@ -21,7 +21,7 @@ public class TwoSqlExpressionEventData : EventData
/// <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(
public TwoSqlExpressionsEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] SqlExpression left,
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,15 +637,15 @@ protected override Expression VisitCollate(CollateExpression collateExpresion)
}

/// <inheritdoc />
protected override Expression VisitDistinctSql(DistinctSqlExpression distinctSqlExpression)
protected override Expression VisitDistinct(DistinctExpression distinctExpression)
{
Check.NotNull(distinctSqlExpression, nameof(distinctSqlExpression));
Check.NotNull(distinctExpression, nameof(distinctExpression));

_relationalCommandBuilder.Append("DISTINCT (");
Visit(distinctSqlExpression.Operand);
Visit(distinctExpression.Operand);
_relationalCommandBuilder.Append(")");

return distinctSqlExpression;
return distinctExpression;
}

/// <inheritdoc />
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ public virtual SqlExpression TranslateAverage([NotNull] SqlExpression sqlExpress
if (inputType == typeof(int)
|| inputType == typeof(long))
{
sqlExpression = sqlExpression is DistinctSqlExpression distinctSqlExpression
? new DistinctSqlExpression(_sqlExpressionFactory.ApplyDefaultTypeMapping(
_sqlExpressionFactory.Convert(distinctSqlExpression.Operand, typeof(double))))
sqlExpression = sqlExpression is DistinctExpression distinctExpression
? new DistinctExpression(_sqlExpressionFactory.ApplyDefaultTypeMapping(
_sqlExpressionFactory.Convert(distinctExpression.Operand, typeof(double))))
: _sqlExpressionFactory.ApplyDefaultTypeMapping(
_sqlExpressionFactory.Convert(sqlExpression, typeof(double)));
}
Expand Down Expand Up @@ -623,7 +623,7 @@ SqlExpression GetExpressionForAggregation(GroupingElementExpression groupingElem
if (groupingElement.IsDistinct
&& !(selector is SqlFragmentExpression))
{
selector = new DistinctSqlExpression(selector);
selector = new DistinctExpression(selector);
}

return selector;
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public virtual SqlExpression ApplyTypeMapping(SqlExpression sqlExpression, Relat
{
CaseExpression e => ApplyTypeMappingOnCase(e, typeMapping),
CollateExpression e => ApplyTypeMappingOnCollate(e, typeMapping),
DistinctSqlExpression e => ApplyTypeMappingOnDistinctSql(e, typeMapping),
DistinctExpression e => ApplyTypeMappingOnDistinct(e, typeMapping),
LikeExpression e => ApplyTypeMappingOnLike(e),
SqlBinaryExpression e => ApplyTypeMappingOnSqlBinary(e, typeMapping),
SqlUnaryExpression e => ApplyTypeMappingOnSqlUnary(e, typeMapping),
Expand Down Expand Up @@ -111,9 +111,9 @@ private SqlExpression ApplyTypeMappingOnCollate(
CollateExpression collateExpression, RelationalTypeMapping typeMapping)
=> collateExpression.Update(ApplyTypeMapping(collateExpression.Operand, typeMapping));

private SqlExpression ApplyTypeMappingOnDistinctSql(
DistinctSqlExpression distinctSqlExpression, RelationalTypeMapping typeMapping)
=> distinctSqlExpression.Update(ApplyTypeMapping(distinctSqlExpression.Operand, typeMapping));
private SqlExpression ApplyTypeMappingOnDistinct(
DistinctExpression distinctExpression, RelationalTypeMapping typeMapping)
=> distinctExpression.Update(ApplyTypeMapping(distinctExpression.Operand, typeMapping));

private SqlExpression ApplyTypeMappingOnSqlUnary(
SqlUnaryExpression sqlUnaryExpression, RelationalTypeMapping typeMapping)
Expand Down
10 changes: 5 additions & 5 deletions src/EFCore.Relational/Query/SqlExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
case CrossJoinExpression crossJoinExpression:
return VisitCrossJoin(crossJoinExpression);

case DistinctSqlExpression distinctSqlExpression:
return VisitDistinctSql(distinctSqlExpression);
case DistinctExpression distinctExpression:
return VisitDistinct(distinctExpression);

case ExceptExpression exceptExpression:
return VisitExcept(exceptExpression);
Expand Down Expand Up @@ -152,11 +152,11 @@ protected override Expression VisitExtension(Expression extensionExpression)
/// <returns> The modified expression, if it or any subexpression was modified; otherwise, returns the original expression. </returns>
protected abstract Expression VisitCrossJoin([NotNull] CrossJoinExpression crossJoinExpression);
/// <summary>
/// Visits the children of the distinct SQL expression.
/// Visits the children of the distinct expression.
/// </summary>
/// <param name="distinctSqlExpression"> The expression to visit. </param>
/// <param name="distinctExpression"> The expression to visit. </param>
/// <returns> The modified expression, if it or any subexpression was modified; otherwise, returns the original expression. </returns>
protected abstract Expression VisitDistinctSql([NotNull] DistinctSqlExpression distinctSqlExpression);
protected abstract Expression VisitDistinct([NotNull] DistinctExpression distinctExpression);
/// <summary>
/// Visits the children of the except expression.
/// </summary>
Expand Down
20 changes: 10 additions & 10 deletions ...y/SqlExpressions/DistinctSqlExpression.cs → ...uery/SqlExpressions/DistinctExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
/// not used in application code.
/// </para>
/// </summary>
public class DistinctSqlExpression : SqlExpression
public class DistinctExpression : SqlExpression
{
/// <summary>
/// Creates a new instance of the <see cref="DistinctSqlExpression" /> class.
/// Creates a new instance of the <see cref="DistinctExpression" /> class.
/// </summary>
/// <param name="operand"> An expression on which DISTINCT is applied. </param>
public DistinctSqlExpression([NotNull] SqlExpression operand)
public DistinctExpression([NotNull] SqlExpression operand)
: base(operand.Type, operand.TypeMapping)
{
Check.NotNull(operand, nameof(operand));
Expand All @@ -50,12 +50,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
/// </summary>
/// <param name="operand"> The <see cref="Operand"/> property of the result. </param>
/// <returns> This expression if no children changed, or an expression with the updated children. </returns>
public virtual DistinctSqlExpression Update([NotNull] SqlExpression operand)
public virtual DistinctExpression Update([NotNull] SqlExpression operand)
{
Check.NotNull(operand, nameof(operand));

return operand != Operand
? new DistinctSqlExpression(operand)
? new DistinctExpression(operand)
: this;
}

Expand All @@ -73,12 +73,12 @@ protected override void Print(ExpressionPrinter expressionPrinter)
public override bool Equals(object obj)
=> obj != null
&& (ReferenceEquals(this, obj)
|| obj is DistinctSqlExpression distinctSqlExpression
&& Equals(distinctSqlExpression));
|| obj is DistinctExpression distinctExpression
&& Equals(distinctExpression));

private bool Equals(DistinctSqlExpression distinctSqlExpression)
=> base.Equals(distinctSqlExpression)
&& Operand.Equals(distinctSqlExpression.Operand);
private bool Equals(DistinctExpression distinctExpression)
=> base.Equals(distinctExpression)
&& Operand.Equals(distinctExpression.Operand);

/// <inheritdoc />
public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), Operand);
Expand Down
16 changes: 8 additions & 8 deletions src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ CollateExpression collateExpression
=> VisitCollate(collateExpression, allowOptimizedExpansion, out nullable),
ColumnExpression columnExpression
=> VisitColumn(columnExpression, allowOptimizedExpansion, out nullable),
DistinctSqlExpression distinctSqlExpression
=> VisitDistinctSql(distinctSqlExpression, allowOptimizedExpansion, out nullable),
DistinctExpression distinctExpression
=> VisitDistinct(distinctExpression, allowOptimizedExpansion, out nullable),
ExistsExpression existsExpression
=> VisitExists(existsExpression, allowOptimizedExpansion, out nullable),
InExpression inExpression
Expand Down Expand Up @@ -505,18 +505,18 @@ protected virtual SqlExpression VisitColumn(
}

/// <summary>
/// Visits a <see cref="DistinctSqlExpression"/> and computes its nullability.
/// Visits a <see cref="DistinctExpression"/> and computes its nullability.
/// </summary>
/// <param name="distinctSqlExpression"> A collate expression to visit. </param>
/// <param name="distinctExpression"> A collate expression to visit. </param>
/// <param name="allowOptimizedExpansion"> A bool value indicating if optimized expansion which considers null value as false value is allowed. </param>
/// <param name="nullable"> A bool value indicating whether the sql expression is nullable. </param>
/// <returns> An optimized sql expression. </returns>
protected virtual SqlExpression VisitDistinctSql(
[NotNull] DistinctSqlExpression distinctSqlExpression, bool allowOptimizedExpansion, out bool nullable)
protected virtual SqlExpression VisitDistinct(
[NotNull] DistinctExpression distinctExpression, bool allowOptimizedExpansion, out bool nullable)
{
Check.NotNull(distinctSqlExpression, nameof(distinctSqlExpression));
Check.NotNull(distinctExpression, nameof(distinctExpression));

return distinctSqlExpression.Update(Visit(distinctSqlExpression.Operand, out nullable));
return distinctExpression.Update(Visit(distinctExpression.Operand, out nullable));
}

/// <summary>
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,16 @@ protected override Expression VisitColumn(ColumnExpression columnExpression)
/// 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>
protected override Expression VisitDistinctSql(DistinctSqlExpression distinctSqlExpression)
protected override Expression VisitDistinct(DistinctExpression distinctExpression)
{
Check.NotNull(distinctSqlExpression, nameof(distinctSqlExpression));
Check.NotNull(distinctExpression, nameof(distinctExpression));

var parentSearchCondition = _isSearchCondition;
_isSearchCondition = false;
var operand = (SqlExpression)Visit(distinctSqlExpression.Operand);
var operand = (SqlExpression)Visit(distinctExpression.Operand);
_isSearchCondition = parentSearchCondition;

return ApplyConversion(distinctSqlExpression.Update(operand), condition: false);
return ApplyConversion(distinctExpression.Update(operand), condition: false);
}

/// <summary>
Expand Down
11 changes: 6 additions & 5 deletions src/EFCore/Diagnostics/CoreEventId.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ private enum Id
RowLimitingOperationWithoutOrderByWarning,
FirstWithoutOrderByAndFilterWarning,
Obsolete_QueryModelOptimized,
NavigationIncluded,
Obsolete_NavigationIncluded,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename the other unused members

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in query there are no unused members. Did you have something specific in mind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Search for // Unused

Obsolete_IncludeIgnoredWarning,
QueryExecutionPlanned,
PossibleUnintendedCollectionNavigationNullComparisonWarning,
PossibleUnintendedReferenceComparisonWarning,
InvalidIncludePathError,
QueryCompilationStarting,
NavigationBaseIncluded,

// Infrastructure events
SensitiveDataLoggingEnabledWarning = CoreBaseId + 400,
Expand Down Expand Up @@ -233,17 +234,17 @@ public static readonly EventId QueryCompilationStarting

/// <summary>
/// <para>
/// A navigation was included in the query.
/// A navigation base was included in the query.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="QueryExpressionEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// This event uses the <see cref="NavigationBaseEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId NavigationIncluded
= MakeQueryId(Id.NavigationIncluded);
public static readonly EventId NavigationBaseIncluded
= MakeQueryId(Id.NavigationBaseIncluded);

/// <summary>
/// <para>
Expand Down
52 changes: 9 additions & 43 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,15 @@ private static string RowLimitingOperationWithoutOrderByWarning(EventDefinitionB
}

/// <summary>
/// Logs for the <see cref="CoreEventId.NavigationIncluded" /> event.
/// Logs for the <see cref="CoreEventId.NavigationBaseIncluded" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation being included. </param>
public static void NavigationIncluded(
public static void NavigationBaseIncluded(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
[NotNull] INavigation navigation)
[NotNull] INavigationBase navigation)
{
var definition = CoreResources.LogNavigationIncluded(diagnostics);
var definition = CoreResources.LogNavigationBaseIncluded(diagnostics);

if (diagnostics.ShouldLog(definition))
{
Expand All @@ -393,54 +393,20 @@ public static void NavigationIncluded(

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new NavigationEventData(
var eventData = new NavigationBaseEventData(
definition,
NavigationIncluded,
NavigationBaseIncluded,
navigation);

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

private static string NavigationIncluded(EventDefinitionBase definition, EventData payload)
private static string NavigationBaseIncluded(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.DeclaringEntityType.ShortName() + "." + p.Navigation.Name);
}

/// <summary>
/// Logs for the <see cref="CoreEventId.NavigationIncluded" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="skipNavigation"> The navigation being included. </param>
public static void NavigationIncluded(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
[NotNull] ISkipNavigation skipNavigation)
{
var definition = CoreResources.LogNavigationIncluded(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, skipNavigation.DeclaringEntityType.ShortName() + "." + skipNavigation.Name);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new SkipNavigationEventData(
definition,
SkipNavigationIncluded,
skipNavigation);

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

private static string SkipNavigationIncluded(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string>)definition;
var p = (SkipNavigationEventData)payload;
return d.GenerateMessage(p.Navigation.DeclaringEntityType.ShortName() + "." + p.Navigation.Name);
var p = (NavigationBaseEventData)payload;
return d.GenerateMessage(p.NavigationBase.DeclaringEntityType.ShortName() + "." + p.NavigationBase.Name);
}

/// <summary>
Expand Down
Loading