diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQuerySqlGenerator.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQuerySqlGenerator.cs index 53466ffa814..afa6941c44a 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQuerySqlGenerator.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQuerySqlGenerator.cs @@ -16,9 +16,28 @@ public class CosmosQuerySqlGenerator(ITypeMappingSource typeMappingSource) : Sql { private readonly IndentedStringBuilder _sqlBuilder = new(); private IReadOnlyDictionary _parameterValues = null!; + + /// + /// The Cosmos SqlParameters which will get sent in the CosmosQuery. + /// private List _sqlParameters = null!; + + /// + /// Lookup table from (the "original" name) to the Cosmos , + /// whose name has underdone uniquification and prefixing. This is used when the same parameter is referenced multiple times in the query. + /// + private Dictionary _sqlParametersByOriginalName = null!; + + /// + /// Contains final parameter names (prefixed, uniquified) seen so far, for uniquification purposes. + /// + private readonly HashSet _prefixedParameterNames = new(StringComparer.OrdinalIgnoreCase); + private ParameterNameGenerator _parameterNameGenerator = null!; + private static readonly bool UseOldBehavior37252 = + AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37252", out var enabled) && enabled; + /// /// 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 @@ -32,6 +51,7 @@ public virtual CosmosSqlQuery GetSqlQuery( _sqlBuilder.Clear(); _parameterValues = parameterValues; _sqlParameters = []; + _sqlParametersByOriginalName = []; _parameterNameGenerator = new ParameterNameGenerator(); Visit(selectExpression); @@ -401,7 +421,11 @@ when _parameterValues.TryGetValue(queryParameter.Name, out var parameterValue) substitutions = new string[parameterValues.Length]; for (var i = 0; i < parameterValues.Length; i++) { - var parameterName = _parameterNameGenerator.GenerateNext(); + // Note that we don't go through _sqlParametersByOriginalName, since the FromSql parameters we're adding here cannot + // be referenced multiple times. + var parameterName = UseOldBehavior37252 + ? _parameterNameGenerator.GenerateNext() + : PrefixAndUniquifyParameterName("p"); _sqlParameters.Add(new SqlParameter(parameterName, parameterValues[i])); substitutions[i] = parameterName; } @@ -677,20 +701,42 @@ protected override Expression VisitSqlConditional(SqlConditionalExpression sqlCo /// protected override Expression VisitSqlParameter(SqlParameterExpression sqlParameterExpression) { - var parameterName = $"@{sqlParameterExpression.Name}"; - - if (_sqlParameters.All(sp => sp.Name != parameterName)) + if (UseOldBehavior37252) { - Check.DebugAssert(sqlParameterExpression.TypeMapping is not null, "SqlParameterExpression without a type mapping."); + var parameterName = $"@{sqlParameterExpression.Name}"; + + if (_sqlParameters.All(sp => sp.Name != parameterName)) + { + Check.DebugAssert(sqlParameterExpression.TypeMapping is not null, "SqlParameterExpression without a type mapping."); + + _sqlParameters.Add( + new SqlParameter( + parameterName, + ((CosmosTypeMapping)sqlParameterExpression.TypeMapping) + .GenerateJToken(_parameterValues[sqlParameterExpression.Name]))); + } - _sqlParameters.Add( - new SqlParameter( - parameterName, - ((CosmosTypeMapping)sqlParameterExpression.TypeMapping) - .GenerateJToken(_parameterValues[sqlParameterExpression.Name]))); + _sqlBuilder.Append(parameterName); } + else + { + if (!_sqlParametersByOriginalName.TryGetValue(sqlParameterExpression.Name, out var sqlParameter)) + { + Check.DebugAssert(sqlParameterExpression.TypeMapping is not null, "SqlParameterExpression without a type mapping."); + + var parameterName = PrefixAndUniquifyParameterName(sqlParameterExpression.Name); - _sqlBuilder.Append(parameterName); + sqlParameter = new SqlParameter( + parameterName, + ((CosmosTypeMapping)sqlParameterExpression.TypeMapping) + .GenerateJToken(_parameterValues[sqlParameterExpression.Name])); + + _sqlParametersByOriginalName[sqlParameterExpression.Name] = sqlParameter; + _sqlParameters.Add(sqlParameter); + } + + _sqlBuilder.Append(sqlParameter.Name); + } return sqlParameterExpression; } @@ -863,6 +909,20 @@ private void GenerateFunction(string name, IReadOnlyList arguments) _sqlBuilder.Append(')'); } + private string PrefixAndUniquifyParameterName(string originalName) + { + var i = 0; + var prefixedName = $"@{originalName}"; + + while (_prefixedParameterNames.Contains(prefixedName)) + { + prefixedName = $"@{originalName + i++}"; + } + + _prefixedParameterNames.Add(prefixedName); + return prefixedName; + } + private sealed class ParameterNameGenerator { private int _count; diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/FromSqlQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/FromSqlQueryCosmosTest.cs index eb9fbe5ee80..43b68cf565e 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/FromSqlQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/FromSqlQueryCosmosTest.cs @@ -357,12 +357,12 @@ public Task FromSqlRaw_queryable_with_parameters(bool async) AssertSql( """ -@p0='London' -@p1='Sales Representative' +@p='London' +@p0='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p AND c["ContactTitle"] = @p0 ) s """); }); @@ -388,12 +388,12 @@ public Task FromSqlRaw_queryable_with_parameters_inline(bool async) AssertSql( """ -@p0='London' -@p1='Sales Representative' +@p='London' +@p0='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p AND c["ContactTitle"] = @p0 ) s """); }); @@ -418,11 +418,11 @@ public Task FromSqlRaw_queryable_with_null_parameter(bool async) AssertSql( """ -@p0=null +@p=null SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Employee" AND c["ReportsTo"] = @p0 OR (IS_NULL(c["ReportsTo"]) AND IS_NULL(@p0)) + SELECT * FROM root c WHERE c["$type"] = "Employee" AND c["ReportsTo"] = @p OR (IS_NULL(c["ReportsTo"]) AND IS_NULL(@p)) ) s """); }); @@ -451,12 +451,12 @@ public Task FromSqlRaw_queryable_with_parameters_and_closure(bool async) AssertSql( """ -@p0='London' +@p='London' @contactTitle='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p0 + SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p ) s WHERE (s["ContactTitle"] = @contactTitle) """); @@ -540,22 +540,22 @@ public virtual Task FromSqlRaw_queryable_with_parameters_cache_key_includes_para AssertSql( """ -@p0='London' -@p1='Sales Representative' +@p='London' +@p0='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p AND c["ContactTitle"] = @p0 ) s """, // """ -@p0='Madrid' -@p1='Accounting Manager' +@p='Madrid' +@p0='Accounting Manager' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["$type"] = "Customer" AND c["City"] = @p AND c["ContactTitle"] = @p0 ) s """); }); @@ -731,12 +731,12 @@ await AssertQuery( AssertSql( """ -@p0='London' -@p1='Sales Representative' +@p='London' +@p0='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["City"] = @p AND c["ContactTitle"] = @p0 ) s """); }); @@ -754,13 +754,50 @@ await AssertQuery( AssertSql( """ -@p0='London' -@p1='Sales Representative' +@p='London' +@p0='Sales Representative' SELECT VALUE s FROM ( - SELECT * FROM root c WHERE c["City"] = @p0 AND c["ContactTitle"] = @p1 + SELECT * FROM root c WHERE c["City"] = @p AND c["ContactTitle"] = @p0 ) s +"""); + }); + + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual Task Both_FromSql_and_regular_parameters(bool async) + => Fixture.NoSyncTest( + async, async a => + { + var city = "London"; + var country = "UK"; + + await AssertQuery( + a, + ss => ((DbSet)ss.Set()) + .FromSql($"""SELECT * FROM root c WHERE c["City"] = {city} AND c["Country"] = {country}""") + .OrderBy(c => c.CustomerID) + .Skip(1) // Non-lambda parameter, so becomes a SqlParameter + .Take(5), + ss => ss.Set() + .Where(x => x.City == city) + .OrderBy(c => c.CustomerID) + .Skip(1) + .Take(5)); + + AssertSql( + """ +@p='London' +@p0='UK' +@p00='1' +@p1='5' + +SELECT VALUE s +FROM ( + SELECT * FROM root c WHERE c["City"] = @p AND c["Country"] = @p0 +) s +ORDER BY s["id"] +OFFSET @p00 LIMIT @p1 """); }); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs index f91ad1dee97..d65ccd5a001 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs @@ -1555,11 +1555,11 @@ public override Task Two_parameters_with_same_case_insensitive_name_get_uniquifi AssertSql( """ @customerID='ANATR' -@customerId='ALFKI' +@customerId0='ALFKI' SELECT VALUE c FROM root c -WHERE ((c["id"] = @customerID) OR (c["id"] = @customerId)) +WHERE ((c["id"] = @customerID) OR (c["id"] = @customerId0)) """); });