[release/10.0] Fix duplicate DbParameter naming uniquification

src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs

@@ -35,12 +35,17 @@ private readonly IDictionary<FromSqlExpression, Expression> _visitedFromSqlExpre
 
     private readonly Dictionary<string, SqlParameterExpression> _sqlParameters = new();
 
+    private Dictionary<DbParameter, RawRelationalParameter>? _processedDbParameters;
+
     private ParametersCacheDecorator _parametersDecorator;
     private ParameterNameGenerator _parameterNameGenerator;
 
     private static readonly bool UseOldBehavior37189 =
         AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37189", out var enabled37189) && enabled37189;
 
+    private static readonly bool UseOldBehavior37409 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37409", out var enabled37409) && enabled37409;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -76,6 +81,7 @@ public virtual Expression Expand(Expression queryExpression, ParametersCacheDeco
         _visitedFromSqlExpressions.Clear();
         _prefixedParameterNames.Clear();
         _sqlParameters.Clear();
+        _processedDbParameters?.Clear();
         _parameterNameGenerator = _parameterNameGeneratorFactory.Create();
         _parametersDecorator = parametersDecorator;
 
@@ -152,8 +158,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
                     {
@@ -206,8 +211,7 @@ object ProcessConstantValue(object? existingConstantValue)
         {
             if (existingConstantValue is DbParameter dbParameter)
             {
-                ProcessDbParameter(dbParameter);
-                return new RawRelationalParameter(dbParameter.ParameterName, dbParameter);
+                return ProcessDbParameter(dbParameter);
             }
 
             return _sqlExpressionFactory.Constant(
@@ -216,10 +220,33 @@ object ProcessConstantValue(object? existingConstantValue)
                 _typeMappingSource.GetMappingForValue(existingConstantValue));
         }
 
-        void ProcessDbParameter(DbParameter dbParameter)
-            => dbParameter.ParameterName = string.IsNullOrEmpty(dbParameter.ParameterName)
+        RawRelationalParameter ProcessDbParameter(DbParameter dbParameter)
+        {
+            if (UseOldBehavior37409)
+            {
+                dbParameter.ParameterName = string.IsNullOrEmpty(dbParameter.ParameterName)
+                    ? GenerateNewParameterName()
+                    : UniquifyParameterName(dbParameter.ParameterName);
+                return new RawRelationalParameter(dbParameter.ParameterName, 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))
+            {
+                return existingParameter;
+            }
+
+            dbParameter.ParameterName = string.IsNullOrEmpty(dbParameter.ParameterName)
                 ? GenerateNewParameterName()
                 : UniquifyParameterName(dbParameter.ParameterName);
+
+            return _processedDbParameters[dbParameter] = new RawRelationalParameter(dbParameter.ParameterName, dbParameter);
+        }
     }
 
     private string GenerateNewParameterName()

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<Customer>)ss.Set<Customer>())
+                .FromSqlRaw(
+                    NormalizeDelimitersInRawString("SELECT * FROM [Customers] WHERE [City] = {0}"),
+                    dbParameter)
+                .GroupBy(c => c.CustomerID)
+                .Select(g => g.FirstOrDefault()),
+            ss => ss.Set<Customer>()
+                .Where(c => c.City == "Seattle")
+                .GroupBy(c => c.CustomerID)
+                .Select(g => g.FirstOrDefault()));
+    }
+
     protected string NormalizeDelimitersInRawString(string sql)
         => Fixture.TestStore.NormalizeDelimitersInRawString(sql);
 

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 =