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

Keep parameter names for inlined queries #23437

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Keep parameter names for inlined queries #23437

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
47 changes: 38 additions & 9 deletions src/EFCore/Query/Internal/ParameterExtractingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class ParameterExtractingExpressionVisitor : ExpressionVisitor
private readonly EvaluatableExpressionFindingExpressionVisitor _evaluatableExpressionFindingExpressionVisitor;
private readonly ContextParameterReplacingExpressionVisitor _contextParameterReplacingExpressionVisitor;

private readonly Dictionary<Expression, Expression> _evaluatedValues = new(ExpressionEqualityComparer.Instance);
private readonly Dictionary<Expression, EvaluatedValues> _evaluatedValues = new(ExpressionEqualityComparer.Instance);

private IDictionary<Expression, bool> _evaluatableExpressions;
private IQueryProvider? _currentQueryProvider;
Expand Down Expand Up @@ -76,6 +76,11 @@ public ParameterExtractingExpressionVisitor(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual Expression ExtractParameters([NotNull] Expression expression)
{
return ExtractParameters(expression, clearEvaluatedValues: true);
}

private Expression ExtractParameters([NotNull] Expression expression, bool clearEvaluatedValues)
{
var oldEvaluatableExpressions = _evaluatableExpressions;
_evaluatableExpressions = _evaluatableExpressionFindingExpressionVisitor.Find(expression);
Expand All @@ -87,7 +92,10 @@ public virtual Expression ExtractParameters([NotNull] Expression expression)
finally
{
_evaluatableExpressions = oldEvaluatableExpressions;
_evaluatedValues.Clear();
if (clearEvaluatedValues)
{
_evaluatedValues.Clear();
}
}
}

Expand Down Expand Up @@ -280,30 +288,43 @@ private static Expression GenerateConstantExpression(object? value, Type returnT

private Expression Evaluate(Expression expression, bool generateParameter)
{
object? parameterValue;
string? parameterName;
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
if (_evaluatedValues.TryGetValue(expression, out var cachedValue))
{
return cachedValue;
}
var existingExpression = generateParameter ? cachedValue.Parameter : cachedValue.Constant;
if (existingExpression != null)
{
return existingExpression;
}

var parameterValue = GetValue(expression, out var parameterName);
parameterValue = cachedValue.Value;
parameterName = cachedValue.CandidateParameterName;
}
else
{
parameterValue = GetValue(expression, out parameterName);
cachedValue = new EvaluatedValues { CandidateParameterName = parameterName, Value = parameterValue };
_evaluatedValues[expression] = cachedValue;
}

if (parameterValue is IQueryable innerQueryable)
{
return ExtractParameters(innerQueryable.Expression);
return ExtractParameters(innerQueryable.Expression, clearEvaluatedValues: false);
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
}

if (parameterName?.StartsWith(QueryFilterPrefix, StringComparison.Ordinal) != true)
{
if (parameterValue is Expression innerExpression)
{
return ExtractParameters(innerExpression);
return ExtractParameters(innerExpression, clearEvaluatedValues: false);
}

if (!generateParameter)
{
var constantValue = GenerateConstantExpression(parameterValue, expression.Type);

_evaluatedValues.Add(expression, constantValue);
cachedValue.Constant = constantValue;

return constantValue;
}
Expand Down Expand Up @@ -337,7 +358,7 @@ var compilerPrefixIndex

var parameter = Expression.Parameter(expression.Type, parameterName);

_evaluatedValues.Add(expression, parameter);
cachedValue.Parameter = parameter;

return parameter;
}
Expand Down Expand Up @@ -645,5 +666,13 @@ private static bool IsQueryableMethod(Expression expression)
=> expression is MethodCallExpression methodCallExpression
&& methodCallExpression.Method.DeclaringType == typeof(Queryable);
}

private sealed class EvaluatedValues
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
{
public string? CandidateParameterName { get; set; }
public object? Value { get; set; }
public Expression? Constant { get; set; }
public Expression? Parameter { get; set; }
}
}
}
29 changes: 29 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6684,6 +6684,35 @@ public virtual Task Query_reusing_parameter_doesnt_declare_duplicate_parameter(b
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Query_reusing_parameter_with_inner_query_doesnt_declare_duplicate_parameter(bool async)
{
var squadId = 1;

return AssertQuery(
async,
ss =>
{
var innerQuery = ss.Set<Squad>().Where(s => s.Id == squadId);
var outerQuery = ss.Set<Gear>().Where(g => innerQuery.Contains(g.Squad));
return outerQuery.Concat(outerQuery).OrderBy(g => g.FullName);
},
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Query_reusing_parameter_with_inner_query_expression_doesnt_declare_duplicate_parameter(bool async)
{
var gearId = 1;
Expression<Func<Gear, bool>> predicate = s => s.SquadId == gearId;

return AssertQuery(
async,
ss => ss.Set<Squad>().Where(s => s.Members.AsQueryable().Where(predicate).Where(predicate).Any()));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Query_reusing_parameter_doesnt_declare_duplicate_parameter_complex(bool async)
Expand Down
45 changes: 44 additions & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6281,6 +6281,49 @@ FROM [Gears] AS [g]
ORDER BY [t].[FullName]");
}

public override async Task Query_reusing_parameter_with_inner_query_doesnt_declare_duplicate_parameter(bool async)
{
await base.Query_reusing_parameter_with_inner_query_doesnt_declare_duplicate_parameter(async);

AssertSql(
@"@__squadId_0='1'

SELECT [t].[Nickname], [t].[SquadId], [t].[AssignedCityName], [t].[CityOfBirthName], [t].[Discriminator], [t].[FullName], [t].[HasSoulPatch], [t].[LeaderNickname], [t].[LeaderSquadId], [t].[Rank]
FROM (
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
FROM [Gears] AS [g]
INNER JOIN [Squads] AS [s] ON [g].[SquadId] = [s].[Id]
WHERE EXISTS (
SELECT 1
FROM [Squads] AS [s0]
WHERE ([s0].[Id] = @__squadId_0) AND ([s0].[Id] = [s].[Id]))
UNION ALL
SELECT [g0].[Nickname], [g0].[SquadId], [g0].[AssignedCityName], [g0].[CityOfBirthName], [g0].[Discriminator], [g0].[FullName], [g0].[HasSoulPatch], [g0].[LeaderNickname], [g0].[LeaderSquadId], [g0].[Rank]
FROM [Gears] AS [g0]
INNER JOIN [Squads] AS [s1] ON [g0].[SquadId] = [s1].[Id]
WHERE EXISTS (
SELECT 1
FROM [Squads] AS [s2]
WHERE ([s2].[Id] = @__squadId_0) AND ([s2].[Id] = [s1].[Id]))
) AS [t]
ORDER BY [t].[FullName]");
}

public override async Task Query_reusing_parameter_with_inner_query_expression_doesnt_declare_duplicate_parameter(bool async)
{
await base.Query_reusing_parameter_with_inner_query_expression_doesnt_declare_duplicate_parameter(async);

AssertSql(
@"@__gearId_0='1'

SELECT [s].[Id], [s].[Banner], [s].[Banner5], [s].[InternalNumber], [s].[Name]
FROM [Squads] AS [s]
WHERE EXISTS (
SELECT 1
FROM [Gears] AS [g]
WHERE (([s].[Id] = [g].[SquadId]) AND ([g].[SquadId] = @__gearId_0)) AND ([g].[SquadId] = @__gearId_0))");
}

public override async Task Query_reusing_parameter_doesnt_declare_duplicate_parameter_complex(bool async)
{
await base.Query_reusing_parameter_doesnt_declare_duplicate_parameter_complex(async);
Expand Down Expand Up @@ -7094,7 +7137,7 @@ public override async Task Constant_enum_with_same_underlying_value_as_previousl
AssertSql(
@"@__p_0='1'

SELECT TOP(@__p_0) [g].[Rank] & @__p_0
SELECT TOP(@__p_0) [g].[Rank] & 1
FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}
Expand Down
52 changes: 51 additions & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7322,6 +7322,56 @@ LEFT JOIN [Officers] AS [o] ON ([g].[Nickname] = [o].[Nickname]) AND ([g].[Squad
ORDER BY [t].[FullName]");
}

public override async Task Query_reusing_parameter_with_inner_query_doesnt_declare_duplicate_parameter(bool async)
{
await base.Query_reusing_parameter_with_inner_query_doesnt_declare_duplicate_parameter(async);

AssertSql(
@"@__squadId_0='1'

SELECT [t].[Nickname], [t].[SquadId], [t].[AssignedCityName], [t].[CityOfBirthName], [t].[FullName], [t].[HasSoulPatch], [t].[LeaderNickname], [t].[LeaderSquadId], [t].[Rank], [t].[Discriminator]
FROM (
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], CASE
WHEN [o].[Nickname] IS NOT NULL THEN N'Officer'
END AS [Discriminator]
FROM [Gears] AS [g]
LEFT JOIN [Officers] AS [o] ON ([g].[Nickname] = [o].[Nickname]) AND ([g].[SquadId] = [o].[SquadId])
INNER JOIN [Squads] AS [s] ON [g].[SquadId] = [s].[Id]
WHERE EXISTS (
SELECT 1
FROM [Squads] AS [s0]
WHERE ([s0].[Id] = @__squadId_0) AND ([s0].[Id] = [s].[Id]))
UNION ALL
SELECT [g0].[Nickname], [g0].[SquadId], [g0].[AssignedCityName], [g0].[CityOfBirthName], [g0].[FullName], [g0].[HasSoulPatch], [g0].[LeaderNickname], [g0].[LeaderSquadId], [g0].[Rank], CASE
WHEN [o0].[Nickname] IS NOT NULL THEN N'Officer'
END AS [Discriminator]
FROM [Gears] AS [g0]
LEFT JOIN [Officers] AS [o0] ON ([g0].[Nickname] = [o0].[Nickname]) AND ([g0].[SquadId] = [o0].[SquadId])
INNER JOIN [Squads] AS [s1] ON [g0].[SquadId] = [s1].[Id]
WHERE EXISTS (
SELECT 1
FROM [Squads] AS [s2]
WHERE ([s2].[Id] = @__squadId_0) AND ([s2].[Id] = [s1].[Id]))
) AS [t]
ORDER BY [t].[FullName]");
}

public override async Task Query_reusing_parameter_with_inner_query_expression_doesnt_declare_duplicate_parameter(bool async)
{
await base.Query_reusing_parameter_with_inner_query_expression_doesnt_declare_duplicate_parameter(async);

AssertSql(
@"@__gearId_0='1'

SELECT [s].[Id], [s].[Banner], [s].[Banner5], [s].[InternalNumber], [s].[Name]
FROM [Squads] AS [s]
WHERE EXISTS (
SELECT 1
FROM [Gears] AS [g]
LEFT JOIN [Officers] AS [o] ON ([g].[Nickname] = [o].[Nickname]) AND ([g].[SquadId] = [o].[SquadId])
WHERE (([s].[Id] = [g].[SquadId]) AND ([g].[SquadId] = @__gearId_0)) AND ([g].[SquadId] = @__gearId_0))");
}

public override async Task Query_reusing_parameter_doesnt_declare_duplicate_parameter_complex(bool async)
{
await base.Query_reusing_parameter_doesnt_declare_duplicate_parameter_complex(async);
Expand Down Expand Up @@ -8242,7 +8292,7 @@ public override async Task Constant_enum_with_same_underlying_value_as_previousl
AssertSql(
@"@__p_0='1'

SELECT TOP(@__p_0) [g].[Rank] & @__p_0
SELECT TOP(@__p_0) [g].[Rank] & 1
FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}
Expand Down