Runtime: LINQ: Consider adding overloads that capture variables for operators that currently don't

[divega]

Many LINQ operators on Enumerable and Queryable take arguments of some Func<> type (or Expression<Func<>> in the case of Queryable) which allow them to capture variables. This results in the following behavior:

``` C#
var letter = "";
var q = from db.Beattles.Where(p => p.Name.StartsWith(letter)); // variable letter gets captured
letter = "P";
var beattle1 = q.First(); // Returns Paul

letter = "J";
var beattle2 = q.First(); // Returns John

A few operators like `Skip`, `Take`, `ElementAt`, `DefaultIfEmpty`, `Aggregate` and `Contains` (#13842 proposes a couple of additions) however accept arguments of simple types. Unless these operators appear nested inside other expressions, variables won't be captured but values. This pattern results in the following behavior:

``` C#
var position = 1;
var q = from db.Orders.Skip(position - 1); // a value of 0 gets captured

position = 10;
var tenth = q.First(); // returns the first Order
position = 20;
var twentieth = q.First(); // still returns the first Order

Each of these behaviors can be seen as correct, but IMO the fact that they are different can be subtle and confusing unless you know about the closures.

A possible way to address this is to add overloads that capture, e.g. for Enumerable.Skip(int) the signature of the new overload would be:

``` C#
public static IEnumerable Skip(
this IEnumerable source,
Func countAccessor)

Usage would be as follows:

``` C#
var position = 1;
var q = from db.Orders.Skip(() => position - 1); 

position = 10;
var tenth = q.First(); 
position = 20;
var twentieth = q.First(); 

Another side effect manifests in specific translating LINQ providers such as the one contained in EF6 in which whether a value or a variable is captured influences whether a literal or a parameter is generated in the resulting SQL query.

This simple rule enables some control over the SQL that gests generated but in the past it was the reason that repeated execution of queries that contained paging (i.e. Skip and Take) operations would pollute the database query cache (see http://stackoverflow.com/questions/9201403/force-entity-framework-to-use-sql-parameterization-for-better-sql-proc-cache-reu for more details). For this reason EF6 already defines overloads of Skip() and Take() that accept Expression<Func<int>> arguments.

[svick]

IMO the fact that they are different can be subtle and confusing unless you know about the closures

Since you're not proposing to remove the old overloads (because that would be a breaking change), I think adding the new overloads does not make this less subtle and only adds confusion ("What is the difference between the two overloads?").

For this reason EF6 already defines overloads of Skip() and Take() that accept Expression<Func<int>> arguments.

Is this specific to EF, or do other queryable providers face the same issue and would welcome the same solution? Maybe keeping these overloads EF-specific is the right choice here?

[divega]

@svick I agree that given the existing overloads cannot be removed the proposal is just a compromise. The potential confusion is a fair concern, but I don't know of an objective way to judge if the added overloads would be more confusing that the current state of affairs. At least if the overloads existed we could point customers to them when needed.

I am not sure if other LINQ to database providers have attempted to use a simple rule to allow users to control parametrization like EF does. Given the overloads don't exist, there aren't many options but to employ a more complicated set of rules (we ended up doing this in EF Core), to add the overloads yourself, or to generate less efficient queries. In any case, I tried to explain that the EF issue is an additional side effect of the current design, not saying that it is per se enough to justify a change.

[nydehi]

The easiest way which is done in apis is to define a new operator jump with
the new behavior and deprecate not remove skip. no breaking change for 5
years or so.

On Dec 6, 2016 3:41 AM, "Diego Vega" [e-mail] wrote:

@svick https://github.com/svick I agree that given the existing
overloads cannot be removed the proposal is just a compromise. The
potential confusion is a fair concern, but I don't know of an objective way
to judge if the added overloads would be more confusing that the current
state of affairs. At least if the overloads existed we could point
customers to them when needed.

I am not sure if other LINQ to database providers have attempted to use a
simple rule to allow users to control parametrization like EF does. Given
the overloads don't exist, there aren't many options but to employ a more
complicated set of rules (we ended up doing this in EF Core), to add the
overloads yourself, or to generate less efficient queries. In any case, I
tried to explain that the EF issue is an additional side effect of the
current design, not saying that it is per se enough to justify a change.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/14144#issuecomment-265092368,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB5LFsg7fzGZPzK7Iv9YTO0_0QEAW47jks5rFR-ngaJpZM4LA4T3
.

[jnm2]

I assume this is referencing https://github.com/aspnet/EntityFramework/issues/3726?

Like I said there, please don't add Expression<Func<int>> overloads if there is any way to avoid it. It's a weird abstraction that is arbitrary and in my opinion leaky. If I understand correctly, it's possible for Entity Framework to parameterize without these it and the result would be worth it.

[eiriktsarpalis]

Using closures that capture mutable state is considered bad practice, and as such exposing an overload dedicated to facilitating this for Skip seems odd to me. I don't think we could accept this, so I'm going to close this issue. Feel free to continue the conversation or reopen the issue.

[divega]

@eiriktsarpalis, I think it is fine to close the issue if there are no plans to ever implement the suggestion. However, I have to mention that the argument you made seems a bit strange. These query operators that don't take lambdas are the odd ones. The majority of the methods on Queryable and Enumerable do take lambdas.