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

Compiled query cache issues with ConstantExpression #33673

Open
Ranio321 opened this issue May 3, 2024 · 2 comments
Open

Compiled query cache issues with ConstantExpression #33673

Ranio321 opened this issue May 3, 2024 · 2 comments

Comments

@Ranio321
Copy link

Ranio321 commented May 3, 2024

Lets assume we want to execute 2 queries that contains a constant expression with the same object but with different values inside this object. In provided example query has been created with a help of EF.Const and with a object (value) of type List.

For queryResultWithOneId query has been generated correctly.

Generated query:
SELECT [i].[Id] FROM [Items] AS [i] WHERE [i].[Id] = 1

However after changing contents of idList by adding a value (2) and executing the same query second time, generated query is no longer correct as it is exactly the same as the one generated for queryResultWithOneId.

Generated query:
SELECT [i].[Id] FROM [Items] AS [i] WHERE [i].[Id] = 1

Correct query:
SELECT [i].[Id] FROM [Items] AS [i] WHERE [i].[Id] IN (1, 2)

How to reproduce

using Microsoft.EntityFrameworkCore;

using var context = new TestDbContext();

var idList = new List<int> { 1 };

var queryResultWithOneId = context.Items
    .Where(x => EF.Constant(idList).Contains(x.Id))
    .ToQueryString();

idList.Add(2);

var queryResultWithTwoId = context.Items
    .Where(x => EF.Constant(idList).Contains(x.Id))
    .ToQueryString();

Console.ReadLine();

public class TestDbContext : DbContext
{
    public DbSet<Item> Items { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Blogging;Trusted_Connection=True");
    }
}

public class Item
{
    public int Id { get; set; }
}

Investigation

After a brief investigation, it became apparent that ICompiledQueryCache got a hit and returned a complied query from cache. It seems that ICompiledQueryCacheKeyGenerator generates a key based of created expression. This expression contains ConstantExpression that holds a reference (or value, depending on the provided constant type) to provided value. Consequently, modifying contents of idList also changes value stored in expression stored within cache key. When executing a second query, the query plan cache key has already been altered due to changing idList contents. Subsequently, _memoryCache.TryGetValue in ICompiledQueryCache got a hit and returns compiled query that was created when idList contained old values.

In sumary, changing values in reference types referenced in ConstantExpression changes cache key of previously generated query plan which causes cache to return invalid query plans.

Include provider and version information

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system: Windows 10
IDE: Visual Studio 2022 17.8.6

@roji
Copy link
Member

roji commented May 3, 2024

The query cache comparison likely doesn't check inside collections in ConstantExpression - this is something we can look at.

Though note that we have #33674, which is about reimplementing EF.Constant in a more efficient way, which would also fix this (queries with different values of the list would always have the same cache key, and the actual list would be integrated only later, in the 2nd part of the query pipeline). However, the problem would technically remain for people building expression trees manually and inserting ConstantExpression nodes themselves.

Note: you should be able to work around this by using a different list instance (though this should produce a cache miss for all cases, even those where the collection contents is the same).

@roji roji added this to the Backlog milestone May 3, 2024
@Ranio321
Copy link
Author

Ranio321 commented May 3, 2024

I see. Thank you very much for response.

For anyone interested here is a simple workaround. As Roji mentioned all you have to do is to create a new list instance

using Microsoft.EntityFrameworkCore;

using var context = new TestDbContext();

var idList = new List<int> { 1 };

var queryResultWithOneId = context.Items
    .Where(x => EF.Constant(idList).Contains(x.Id))
    .ToQueryString();

idList.Add(2);

var newList = idList.ToList(); // <----------- Added line

var queryResultWithTwoId = context.Items
    .Where(x => EF.Constant(newList).Contains(x.Id))
    .ToQueryString();

Console.ReadLine();

public class TestDbContext : DbContext
{
    public DbSet<Item> Items { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Blogging;Trusted_Connection=True");
    }
}

public class Item
{
    public int Id { get; set; }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants