Where clause with boolean check isn't always parameterized as expected

[xt0rted]

Steps to reproduce

Resulting query is not parameterized

var users = _db.Users.Where(u => u.IsEnabled).ToList();

// or

var users = _db.Users.Where(u => u.IsEnabled == true).ToList();

SELECT * FROM [Users] AS [u] WHERE ([u].[IsEnabled] = 1);

Resulting query is parameterized

var isEnabled = true;
var users = _db.Users.Where(u => u.IsEnabled == isEnabled).ToList();

DECLARE @__isEnabled_1 bit = 1;
SELECT * FROM [Users] AS [u] WHERE ([u].[IsEnabled] = @__isEnabled_1);

Other methods, such as .Skip() and .Take(), get parameterized, it'd be nice if .Where(u => u.IsEnabled) style checks were as well.

Further technical details

EF Core version: 2.1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win10
IDE: Visual Studio 2017 15.7.6

[ajcvickers]

@xt0rted This is by-design behavior. In all but special cases (such as Skip and Take) constants are not parameterized since the value can't be changed in multiple executions of the same LINQ query. If parameterization is needed in such cases, then the query can be re-written like above to force a parameter.

[xt0rted]

I know this was closed as by design, but with #13617 discussing something similar I wanted to bring this back up. Wouldn't it be better to parameterize the query in my example for the same reasons as mentioned in the other issue?

[divega]

@xt0rted I will try to explain the design:

In most cases, EF Core only generates parameters for variables referenced in the query. The goal of this design is to give you control over whether parameters or constants are used.

Issue #13617 is a degenerate case because it is very hard to write the query in a way that the values in the enumerable passed to Contains are parameterized in the SQL. The same was true with Skip() and Take() because of the way the LINQ operators are defined (they do not take lambda expressions, so they don't capture variables).

In those cases it is common that the same query, constructed and executed multiple times, will end up producing different SQL, and potentially a large number of distinct cache entries if only some of the values passed to the query changed on each execution.

For the first and second queries in this issue, that is not the case: Building and executing the query will result in the same SQL every time. There is really no value to parametrize if you write it using u => u.IsEnabled and the value is a constant in u => u.IsEnabled == true.

If we parameterized a query like that, then a similar query with a predicate u => !u.Enabled would be translated to the same SQL. I.e. you would save one entry in the query cache, assuming that the query with the negated condition in the predicate actually exists in the application.

But parameterization is not always desirable. For example, parameterizing all values would result in the database server having to come up with a single query plan that will work for any values. The database server will likely use heuristics, for example parameter sniffing, to decide the plan, which means that the plan will likely be suboptimal for some values.