Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect parameter rewriting for string StartsWith/EndsWith/Contains #32432

Closed
roji opened this issue Nov 28, 2023 · 3 comments · Fixed by #32433
Closed

Incorrect parameter rewriting for string StartsWith/EndsWith/Contains #32432

roji opened this issue Nov 28, 2023 · 3 comments · Fixed by #32433
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Nov 28, 2023

When a parameter is used as the pattern for StartsWith/EndsWith/Contains, in 8.0.0 we rewrite that parameter to escape any wildchars. However, when the same parameter is reused in two different such methods (e.g. StartsWith and Contains), the same parameter name is used, with the second overwriting the first.

Repro:

await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

var s = "foo";
_ = await ctx.Blogs.Where(b => b.Name.StartsWith(s) || b.Body.Contains(s)).ToListAsync();

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string Body { get; set; }
}

This produces the following query:

Executed DbCommand (48ms) [Parameters=[@__s_0_rewritten='%foo%' (Size = 4000)], CommandType='Text', CommandTimeout='30']
SELECT [b].[Id], [b].[Body], [b].[Name]
FROM [Blogs] AS [b]
WHERE [b].[Name] LIKE @__s_0_rewritten ESCAPE N'\' OR [b].[Body] LIKE @__s_0_rewritten ESCAPE N'\'

The StartsWith here is translated to the first LIKE, which incorrectly uses the same parameter as the Contains, with %foo% instead of foo%.

The workaround is to use two separate variables.

Introduced in #31482. Originally flagged by @stevenpua for PostgreSQL in npgsql/efcore.pg#2994.

@roji
Copy link
Member Author

roji commented Nov 28, 2023

Reopening to consider patching.

@penihel
Copy link

penihel commented Jan 18, 2024

I have this problem.

look my c#
AddCriteria(q => q.Pessoa.NomeCompleto.Contains(Term) || q.Pessoa.NumeroDocumentoIdentificador.StartsWith(Term));

Look the SQL,

Has created 2 parameters, but with the same value. it's wrong
DECLARE @__Term_0_rewritten nvarchar(300) = N'D%'; -- wrong, shoulds starts with percent "%" beacause i used Contains in c#
DECLARE @__Term_0_rewritten_1 nvarchar(100) = N'D%'; -- ok, because i used StartWith in c#

look the where clause
WHERE [p].[Ativo] = CAST(1 AS bit) AND ([p].[NomeCompleto] LIKE @__Term_0_rewritten ESCAPE N'\' OR [p].[NumeroDocumentoIdentificador] LIKE @__Term_0_rewritten_1 ESCAPE N'\')

was expected the parameters values like this

DECLARE @__Term_0_rewritten nvarchar(300) = N'%D%'; -- shoulds starts with percent "%" beacause i used Contains in c#
DECLARE @__Term_0_rewritten_1 nvarchar(100) = N'D%'; -- ok, because i used StartWith in c#

in another words.

The value of @__Term_0_rewritten shold be "LIKE" with N'%D%';`

@penihel
Copy link

penihel commented Jan 23, 2024

i think its ok , but when 8.0.2 will be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants