Skip to content

Commit

Permalink
Fix MemberExpression funcletization (#33241)
Browse files Browse the repository at this point in the history
Continues #33106
  • Loading branch information
roji authored Mar 5, 2024
1 parent 87da842 commit eb3f9c4
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -738,10 +738,13 @@ protected override Expression VisitLambda<T>(Expression<T> lambda)
protected override Expression VisitMember(MemberExpression member)
{
// Static member access - notably required for EF.Functions, but also for various translations (DateTime.Now).
// Note that this is treated as a captured variable (so will be parameterized), unless the captured variable is init-only.
if (member.Expression is null)
{
_state = IsGenerallyEvaluatable(member)
? State.CreateEvaluatable(typeof(MemberExpression), containsCapturedVariable: false)
? State.CreateEvaluatable(
typeof(MemberExpression),
containsCapturedVariable: member.Member is not FieldInfo { IsInitOnly: true })
: State.NoEvaluatability;
return member;
}
Expand All @@ -766,7 +769,8 @@ protected override Expression VisitMember(MemberExpression member)

if (IsGenerallyEvaluatable(member))
{
_state = State.CreateEvaluatable(typeof(MemberExpression), _state.ContainsCapturedVariable);
// Note that any evaluatable MemberExpression is treated as a captured variable.
_state = State.CreateEvaluatable(typeof(MemberExpression), containsCapturedVariable: true);
return member.Update(expression);
}

Expand Down Expand Up @@ -1718,9 +1722,7 @@ private static StateType CombineStateTypes(StateType stateType1, StateType state
// We don't evaluate as constant if we're not inside a lambda, i.e. in a top-level operator. This is to make sure that
// non-lambda arguments to e.g. Skip/Take are parameterized rather than evaluated as constant, since that would produce
// different SQLs for each value.
|| !_inLambda
|| (evaluatableRoot is MemberExpression member
&& (member.Expression is not null || member.Member is not FieldInfo { IsInitOnly: true })));
|| !_inLambda);

// We have some cases where a node is evaluatable, but only as part of a larger subtree, and should not be evaluated as a tree root.
// For these cases, the node's state has a notEvaluatableAsRootHandler lambda, which we can invoke to make evaluate the node's
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4714,6 +4714,20 @@ FROM root c
""");
}

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

AssertSql(
"""
@__p_0='ALFKI'
SELECT c
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] = @__p_0))
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5776,4 +5776,14 @@ private class MyCustomerDetails
public string CustomerId { get; set; }
public string City { get; set; }
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Static_member_access_gets_parameterized_within_larger_evaluatable(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => c.CustomerID == StaticProperty + "KI"));

private static string StaticProperty
=> "ALF";
}
Original file line number Diff line number Diff line change
Expand Up @@ -7449,6 +7449,20 @@ public override async Task Compiler_generated_local_closure_produces_valid_param
//AssertSql("");
}

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

AssertSql(
"""
@__p_0='ALFKI' (Size = 5) (DbType = StringFixedLength)
SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] = @__p_0
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down

0 comments on commit eb3f9c4

Please sign in to comment.