diff --git a/src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs b/src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs index 03391128a6f..8c51f99e917 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs @@ -35,6 +35,8 @@ private readonly IDictionary _visitedFromSqlExpre private readonly Dictionary _sqlParameters = new(); + private Dictionary? _processedDbParameters; + private ParametersCacheDecorator _parametersDecorator; private ParameterNameGenerator _parameterNameGenerator; @@ -76,6 +78,7 @@ public virtual Expression Expand(Expression queryExpression, ParametersCacheDeco _visitedFromSqlExpressions.Clear(); _prefixedParameterNames.Clear(); _sqlParameters.Clear(); + _processedDbParameters?.Clear(); _parameterNameGenerator = _parameterNameGeneratorFactory.Create(); _parametersDecorator = parametersDecorator; @@ -152,8 +155,7 @@ private FromSqlExpression VisitFromSql(FromSqlExpression fromSql) { if (parameterValues[i] is DbParameter dbParameter) { - ProcessDbParameter(dbParameter); - subParameters.Add(new RawRelationalParameter(dbParameter.ParameterName, dbParameter)); + subParameters.Add(ProcessDbParameter(dbParameter)); } else { @@ -203,23 +205,32 @@ private FromSqlExpression VisitFromSql(FromSqlExpression fromSql) } object ProcessConstantValue(object? existingConstantValue) + => existingConstantValue is DbParameter dbParameter + ? ProcessDbParameter(dbParameter) + : _sqlExpressionFactory.Constant( + existingConstantValue, + existingConstantValue?.GetType() ?? typeof(object), + _typeMappingSource.GetMappingForValue(existingConstantValue)); + + RawRelationalParameter ProcessDbParameter(DbParameter dbParameter) { - if (existingConstantValue is DbParameter dbParameter) + _processedDbParameters ??= []; + + // In some situations, we duplicate SQL tree fragments (e.g. in GroupBy translation). + // If the duplicated SQL happens to contain a FromSqlExpression referencing a DbParameter, that means we have the same + // DbParameter instance referenced multiple times in the tree, and should absolutely not uniquify its name multiple times + // (since we'd modify its name multiple times). See #37409. + if (_processedDbParameters.TryGetValue(dbParameter, out var existingParameter)) { - ProcessDbParameter(dbParameter); - return new RawRelationalParameter(dbParameter.ParameterName, dbParameter); + return existingParameter; } - return _sqlExpressionFactory.Constant( - existingConstantValue, - existingConstantValue?.GetType() ?? typeof(object), - _typeMappingSource.GetMappingForValue(existingConstantValue)); - } - - void ProcessDbParameter(DbParameter dbParameter) - => dbParameter.ParameterName = string.IsNullOrEmpty(dbParameter.ParameterName) + dbParameter.ParameterName = string.IsNullOrEmpty(dbParameter.ParameterName) ? GenerateNewParameterName() : UniquifyParameterName(dbParameter.ParameterName); + + return _processedDbParameters[dbParameter] = new RawRelationalParameter(dbParameter.ParameterName, dbParameter); + } } private string GenerateNewParameterName() diff --git a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs index d25d23e94f2..5b81f8ee2a3 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs @@ -1333,6 +1333,29 @@ public virtual async Task Multiple_occurrences_of_FromSql_with_db_parameter_adds Assert.Empty(actual); } + // The GroupBy followed by a non-reducing Select causes the base FromSql to get duplicated in the SQL, and so the same DbParameter + // to get referenced from multiple FromSqlExpressions. Ensure we only process the DbParameter once. See #37409. + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_GroupBy_non_reducing_Select(bool async) + { + using var context = CreateContext(); + + var dbParameter = CreateDbParameter("city", "Seattle"); + + await AssertQuery( + async, + ss => ((DbSet)ss.Set()) + .FromSqlRaw( + NormalizeDelimitersInRawString("SELECT * FROM [Customers] WHERE [City] = {0}"), + dbParameter) + .GroupBy(c => c.CustomerID) + .Select(g => g.FirstOrDefault()), + ss => ss.Set() + .Where(c => c.City == "Seattle") + .GroupBy(c => c.CustomerID) + .Select(g => g.FirstOrDefault())); + } + protected string NormalizeDelimitersInRawString(string sql) => Fixture.TestStore.NormalizeDelimitersInRawString(sql); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs index 0e62e638b7c..b7edcf2ad91 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs @@ -994,6 +994,35 @@ public override async Task Multiple_occurrences_of_FromSql_with_db_parameter_add """); } + public override async Task FromSql_GroupBy_non_reducing_Select(bool async) + { + await base.FromSql_GroupBy_non_reducing_Select(async); + + AssertSql( + """ +city='Seattle' (Nullable = false) (Size = 7) + +SELECT [m3].[CustomerID], [m3].[Address], [m3].[City], [m3].[CompanyName], [m3].[ContactName], [m3].[ContactTitle], [m3].[Country], [m3].[Fax], [m3].[Phone], [m3].[PostalCode], [m3].[Region] +FROM ( + SELECT [m].[CustomerID] + FROM ( + SELECT * FROM "Customers" WHERE "City" = @city + ) AS [m] + GROUP BY [m].[CustomerID] +) AS [m1] +LEFT JOIN ( + SELECT [m2].[CustomerID], [m2].[Address], [m2].[City], [m2].[CompanyName], [m2].[ContactName], [m2].[ContactTitle], [m2].[Country], [m2].[Fax], [m2].[Phone], [m2].[PostalCode], [m2].[Region] + FROM ( + SELECT [m0].[CustomerID], [m0].[Address], [m0].[City], [m0].[CompanyName], [m0].[ContactName], [m0].[ContactTitle], [m0].[Country], [m0].[Fax], [m0].[Phone], [m0].[PostalCode], [m0].[Region], ROW_NUMBER() OVER(PARTITION BY [m0].[CustomerID] ORDER BY [m0].[CustomerID]) AS [row] + FROM ( + SELECT * FROM "Customers" WHERE "City" = @city + ) AS [m0] + ) AS [m2] + WHERE [m2].[row] <= 1 +) AS [m3] ON [m1].[CustomerID] = [m3].[CustomerID] +"""); + } + public override async Task FromSqlRaw_composed_with_common_table_expression(bool async) { var exception =