Skip to content

Commit 209f088

Browse files
Copilotroji
Copilot
and
roji
authored
[release/10.0] Fix invalid SQL parameter names for switch/case pattern-matched variables (#37805)
Co-authored-by: Shay Rojansky <roji@roji.org>
1 parent aa10d1d commit 209f088

4 files changed

Lines changed: 98 additions & 7 deletions

File tree

‎src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ public class ExpressionTreeFuncletizer : ExpressionVisitor
2525
private static readonly bool UseOldBehavior37152 =
2626
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37152", out var enabled) && enabled;
2727

28+
private static readonly bool UseOldBehavior37465 =
29+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37465", out var enabled37465) && enabled37465;
30+
2831
// The general algorithm here is the following.
2932
// 1. First, for each node type, visit that node's children and get their states (evaluatable, contains evaluatable, no evaluatable).
3033
// 2. Calculate the parent node's aggregate state from its children; a container node whose children are all evaluatable is itself
@@ -2083,12 +2086,19 @@ bool PreserveConvertNode(Expression expression)
20832086

20842087
if (evaluateAsParameter)
20852088
{
2086-
parameterName = tempParameterName ?? "p";
2089+
if (UseOldBehavior37465)
2090+
{
2091+
parameterName = tempParameterName ?? "p";
20872092

2088-
var compilerPrefixIndex = parameterName.LastIndexOf('>');
2089-
if (compilerPrefixIndex != -1)
2093+
var compilerPrefixIndex = parameterName.LastIndexOf('>');
2094+
if (compilerPrefixIndex != -1)
2095+
{
2096+
parameterName = parameterName[(compilerPrefixIndex + 1)..];
2097+
}
2098+
}
2099+
else
20902100
{
2091-
parameterName = parameterName[(compilerPrefixIndex + 1)..];
2101+
parameterName = string.IsNullOrWhiteSpace(tempParameterName) ? "p" : tempParameterName;
20922102
}
20932103

20942104
// The VB compiler prefixes closure member names with $VB$Local_, remove that (#33150)
@@ -2097,6 +2107,17 @@ bool PreserveConvertNode(Expression expression)
20972107
parameterName = parameterName.Substring("$VB$Local_".Length);
20982108
}
20992109

2110+
if (!UseOldBehavior37465)
2111+
{
2112+
// In many databases, parameter names must start with a letter or underscore.
2113+
// The same is true for C# variable names, from which we derive the parameter name, so in principle we shouldn't see an issue;
2114+
// but just in case, prepend an underscore if the parameter name doesn't start with a letter or underscore.
2115+
if (parameterName.Length > 0 && !char.IsLetter(parameterName[0]) && parameterName[0] != '_')
2116+
{
2117+
parameterName = "_" + parameterName;
2118+
}
2119+
}
2120+
21002121
if (UseOldBehavior37152)
21012122
{
21022123
// Uniquify the parameter name
@@ -2162,12 +2183,18 @@ static Expression RemoveConvert(Expression expression)
21622183
switch (memberExpression.Member)
21632184
{
21642185
case FieldInfo fieldInfo:
2165-
parameterName = parameterName is null ? fieldInfo.Name : $"{parameterName}_{fieldInfo.Name}";
2186+
{
2187+
var name = UseOldBehavior37465 ? fieldInfo.Name : SanitizeCompilerGeneratedName(fieldInfo.Name);
2188+
parameterName = parameterName is null ? name : $"{parameterName}_{name}";
21662189
return fieldInfo.GetValue(instanceValue);
2190+
}
21672191

21682192
case PropertyInfo propertyInfo:
2169-
parameterName = parameterName is null ? propertyInfo.Name : $"{parameterName}_{propertyInfo.Name}";
2193+
{
2194+
var name = UseOldBehavior37465 ? propertyInfo.Name : SanitizeCompilerGeneratedName(propertyInfo.Name);
2195+
parameterName = parameterName is null ? name : $"{parameterName}_{name}";
21702196
return propertyInfo.GetValue(instanceValue);
2197+
}
21712198
}
21722199
}
21732200
catch
@@ -2181,7 +2208,9 @@ static Expression RemoveConvert(Expression expression)
21812208
return constantExpression.Value;
21822209

21832210
case MethodCallExpression methodCallExpression:
2184-
parameterName = methodCallExpression.Method.Name;
2211+
parameterName = UseOldBehavior37465
2212+
? methodCallExpression.Method.Name
2213+
: SanitizeCompilerGeneratedName(methodCallExpression.Method.Name);
21852214
break;
21862215

21872216
case UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked } unaryExpression
@@ -2205,6 +2234,25 @@ static Expression RemoveConvert(Expression expression)
22052234
exception);
22062235
}
22072236
}
2237+
2238+
static string SanitizeCompilerGeneratedName(string s)
2239+
{
2240+
// Compiler-generated field names intentionally contain illegal characters, specifically angle brackets <>.
2241+
// In cases where there's something within the angle brackets, that tends to be the original user-provided variable name
2242+
// (e.g. <PropertyName>k__BackingField). If we see angle brackets, extract that out, or if the angle brackets contain no
2243+
// content, strip them out entirely and take what comes after.
2244+
var closingBracket = s.IndexOf('>');
2245+
if (closingBracket == -1)
2246+
{
2247+
return s;
2248+
}
2249+
2250+
var openingBracket = s.IndexOf('<');
2251+
2252+
return openingBracket != -1 && openingBracket < closingBracket - 1
2253+
? s[(openingBracket + 1)..closingBracket]
2254+
: s[(closingBracket + 1)..];
2255+
}
22082256
}
22092257

22102258
private Expression ConvertIfNeeded(Expression expression, Type type)

‎test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4494,6 +4494,18 @@ public override async Task Parameter_extraction_can_throw_exception_from_user_co
44944494
AssertSql();
44954495
}
44964496

4497+
public override Task Captured_variable_from_switch_case_pattern_matching(bool async)
4498+
=> Fixture.NoSyncTest(
4499+
async, async a =>
4500+
{
4501+
await base.Captured_variable_from_switch_case_pattern_matching(a);
4502+
4503+
AssertSql(
4504+
"""
4505+
ReadItem(None, ALFKI)
4506+
""");
4507+
});
4508+
44974509
public override async Task Where_query_composition5(bool async)
44984510
{
44994511
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => AssertQuery(

‎test/EFCore.Specification.Tests/Query/NorthwindMiscellaneousQueryTestBase.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,6 +2993,23 @@ public virtual Task Parameter_extraction_can_throw_exception_from_user_code_2(bo
29932993
&& o.OrderDate.Value.Year == dateFilter.Value.Year)));
29942994
}
29952995

2996+
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
2997+
public virtual async Task Captured_variable_from_switch_case_pattern_matching(bool async)
2998+
{
2999+
object customerIdAsObject = "ALFKI";
3000+
3001+
switch (customerIdAsObject)
3002+
{
3003+
case string customerId:
3004+
{
3005+
await AssertQuery(
3006+
async,
3007+
ss => ss.Set<Customer>().Where(c => c.CustomerID == customerId));
3008+
break;
3009+
}
3010+
}
3011+
}
3012+
29963013
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
29973014
public virtual Task Subquery_member_pushdown_does_not_change_original_subquery_model(bool async)
29983015
=> AssertQuery(

‎test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3236,6 +3236,20 @@ FROM [Orders] AS [o]
32363236
""");
32373237
}
32383238

3239+
public override async Task Captured_variable_from_switch_case_pattern_matching(bool async)
3240+
{
3241+
await base.Captured_variable_from_switch_case_pattern_matching(async);
3242+
3243+
AssertSql(
3244+
"""
3245+
@customerId='ALFKI' (Size = 5) (DbType = StringFixedLength)
3246+
3247+
SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
3248+
FROM [Customers] AS [c]
3249+
WHERE [c].[CustomerID] = @customerId
3250+
""");
3251+
}
3252+
32393253
public override async Task Subquery_member_pushdown_does_not_change_original_subquery_model(bool async)
32403254
{
32413255
await base.Subquery_member_pushdown_does_not_change_original_subquery_model(async);

0 commit comments

Comments
 (0)