Concat iqueryable too many times results on StackOverflowException when ToList is called

[xaviergxf]

This code produces an StackOverflowException when ToList is called on IQueryable. It seems that the expression tree is too long, with too many inner expressions and the generation of sql is done recursively.

Steps to reproduce

var query = dbContext.Users.Take(0).AsQueryable();
foreach(var user in users)
{
    query = query.Concat(dbContext.Users.Where(p=>p.Username == user.Username && p.Email == user.Email));
}
var result = await query.ToList();

When calling ToList() it throws an StackOverflowException.

Further technical details

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Windows 10 1909
IDE: Visual Studio 2019 16.6.3

[ajcvickers]

/cc @roji

[roji]

@xaviergxf how many users are being iterated over here? Dynamically generating queries with query nodes being proportional to data in this way is inherently not a good idea.

The specific query above looks like it could be easily rewritten to use OR conditions instead of Concat - that should probably make the StackOverflowException disappear - but is still probably going to be a bad idea from a perf perspective.

[xaviergxf]

@roji there's thousands of users... I i thought about rewriting it with or, but i would not get a good perf and i would need to use LinqKit for this. Is there anything planed with EF Core to enable a scenario like this one?

[roji]

I i thought about rewriting it with or, but i would not get a good perf and i would need to use LinqKit for this

I highly doubt that Concat would give you better performance than conditions with Or - be sure to measure that carefully (by writing a test raw SQL query and measuring with BenchmarkDotNet), you may be surprised. You also don't require LinqKit to produce dynamic queries like this - you may construct them yourself via System.Linq.Expressions.

However, stepping back, for thousands of users, I'd strongly suggest reviewing your general design beyond this specific query. Regardless of whether you use Concat or conditions with Or, databases and SQL are generally not geared towards massive dynamic queries. Each query would have a unique SQL, and therefore no plans would be cached, etc.

If you can change your query to only check one column (e.g. compare Username only, instead of Username and Email as above), you should be able to use Contains in your query, which EF Core will translate to an SQL IN expression. This also will not provide good perf for thousands of dynamic values, but we at least have some plans to improve that in the future (i.e. #13617). However, a larger redesign may obviate dynamic queries entirely, providing optimal perf. I don't have any context on what you're doing so I can't give further advice...

[ajcvickers]

@smitpatel Were you referring to #11799 or #14661?

[smitpatel]

Both.