Efcore: Discussion about evaluate untranslatable queries on the client

[gdoron]

@rowanmiller @divega Can we please have a discussion about the _we do not know how to translate the code to be evaluated on the server, we'll do it on the client instead_ feature?

I for one, think it's a very risky feature, that will cause people significant performance issues when they'll go live-production.

Instead of evaluate on the client and log a warning (most people I believe won't even see them) I suggest throw an exception and log an error and in the exception itself say "we can't translate your code to a query, you can evaluate the code on the client (e.g. ToList() before your , but beware it can be significantly slow operation on large amount of data" (or something like it).

What do you think?

[smitpatel]

The behavior can be changed with following code in OnConfiguring method
new SqlServerDbContextOptionsBuilder(optionsBuilder).QueryClientEvaluationBehavior(QueryClientEvaluationBehavior.Throw);
Visibility is questionable.

EDIT by @rowanmiller

There is a top level API you can use adjust this setting. The following code can go in OnConfiguring (or in the call to AddDbContext in Startup.cs if you are working in ASP.NET Core)

c# optionsBuilder .UseSqlServer(...) .DisableQueryClientEvaluation();

[gdoron]

@smitpatel

A. I agree it's not visible at all.
B. It should be the default. EF Core should be safe by default, not the other way around.
C. It should be explicit per query, you might not be aware of all the places you'll get hit by this.
D. It shouldn't exist at all IMHO... I think my suggestion is better, forcing the developer on each query to explicitly say he wants to evaluate the query on the client (with ToList, ToArray etc.).

[divega]

Customer feedback can always bring new data and help us revisit issues, change our minds and make better decisions. In this case AFAIR we discussed the default behavior and intentionally landed where we are now. I don't remember all the details of the discussion right now but I am sure we considered all the points above, plus the complexity of writing those queries explicitly as client (which assumes knowledge of what a specific EF provider won't be able to translate), plus supporting providers that won't necessary have the same query expressiveness as SQL databases, plus the fact that client evaluation can be ok if the results are bounded (BTW, we have also considered adding the ability to throw or warn on unbounded results, tracked by #5089 in the backlog).

These two assertions together make me wonder if we need better/stronger guidance on how to take an application into production:

I for one, think it's a very risky feature, that will cause people significant performance issues when they'll go live-production.

Instead of evaluate on the client and log a warning (most people I believe won't even see them)...

Perhaps the solution is to throw by default, but I would certainly recommend all customers to profile their applications and look for warnings on their logs before taking any application to production.

[Suchiman]

I think it's nice that EF can do this now, especially since it can reorder queries to do as much as possible on the server, which isn't the case with forcing ToList. And #5089 seems to perfectly address the major concern about client eval.

But to address the concerns named here, i would suggest to provide a WithPartialClientEvaluation(bool) [_naming WiP_] akin to AsNoTracking(). This provides the greatest flexibility combined with the QueryClientEvaluationBehavior setting.

[gdoron]

The unbound feature is indeed nice, but not enough.
Besides of returning too many results, if only parts of the query will be translated to SQL(or whatever NoSQL language), the server might not use existing indexes.

@Suchiman I'm not sure why you prefer having WithPartialClientEvaluation instead of simply ToList.

[Suchiman]

@gdoron

I think it's nice that EF can do this now, especially since it can reorder queries to do as much as possible on the server, which isn't the case with forcing ToList.

That means that you can have Where(runningOnServer).Where(runningOnClient).Where(runningOnServer) and EF takes care of ordering the query in a fashion that allows the first and last Where to execute on the server while only the second executes on the client. It also abstracts away which parts can actually be translated to the server which may vary by store and version.

Also calling ToList will not allow the result set to be streamed, e.g.
Where(runningOnServer).ToList().Where(runningOnClient).FirstOrDefault()

[gdoron]

@Suchiman

That means that you can have Where(runningOnServer).Where(runningOnClient).Where(runningOnServer) and EF takes care of ordering the query in a fashion that allows the first and last Where to execute on the server while only the second executes on the client.

Since EF builds Expression tree, it doesn't matter (AFAIK) if the conditions are chained with separate Wheres or in a single Where.

It also abstracts away which parts can actually be translated to the server which may vary by store and version.

That's a bad thing. You should be very aware and excplicit about what won't run on the server as it can have a huge impact on the performance.

Also calling ToList will not allow the result set to be streamed

You can then use AsEnumerable instead of ToList

[Suchiman]

@gdoron

Since EF builds Expression tree, it doesn't matter (AFAIK) if the conditions are chained with separate Where s or in a single Where.

Yes correct, my sample is missing a bit of context, it was associated with the second sample:
EF cannot reorder the query if you insert something to force client eval
Where(runningOnServer).AsEnumerable().Where(runningOnClient).Where(couldRunOnServer)

That's a bad thing. You should be very aware and excplicit about what won't run on the server as it can have a huge impact on the performance.

I'll side here with @divega. It's certainly something you should keep an eye on but we could also start forbidding LINQ at all because it doesn't scale in some scenarios and puts intense pressure on the GC.

Having a choice between

• Everything must evaluate server side, even if it requires strong coupling with the data store and raw SQL [QueryClientEvaluationBehavior.Throw, FromSql(string)]
• I decide which exact parts i want to run server side and when i start client eval [QueryClientEvaluationBehavior.Throw, AsEnumerable()]
• I may target multiple stores or client side applications with 5MB SQLite databases are unlikely a bottleneck, let EF choose for me, i can act if there are signs of decreasing performance [AsEnumerable(), WithPartialClientEvaluation(bool)]

doesn't seems like a bad choice for me, considering that EF Core is no longer a "Server only" framework.

[anpete]

@gdoron @smitpatel Just to clarify on the API for disabling client-eval: It can also be set in the options lambda passed to UseSqlServer().

[GSPP]

Client-side evaluation is a very important feature:

1. It provides LINQ 2 SQL compatibility. I am still stuck on L2S with a significant codebase because rewriting all queries to not use this feature is not feasible. I'm hoping for EF7 to provide relief. Please help us abandoned L2S customers to finally migrate away!
2. It is very convenient.

L2S was only capable of evaluating a final Select partially on the client.

query.Select(x => new { x.SomeCol, C = CreateClass(x.Col2) })

automatically became

.Select(x => new { x.SomeCol, x.Col2 }) //Only fetch relevant columns!
.AsEnumerable()
.Select(x => new { x.SomeCol, C = CreateClass(x.Col2) })

This was efficient as it gets and is super convenient when working with MVC view models. Sometimes, you really need to run some helper code to instantiate or compute something.

[rowanmiller]

So far, we still feel like we have the right defaults.

If you don't like the default, then it can easily be changed. The following code can go in OnConfiguring (or in the call to AddDbContext in Startup.cs if you are working in ASP.NET Core)

c# optionsBuilder .UseSqlServer(...) .DisableQueryClientEvaluation();

Agreed that there should be a way to specify whether client eval is OK/not-OK on a per-query basis - this will most likely be post v1.0.0.

Leaving this issue open a little longer to allow further discussion.

[smitpatel]

@rowanmiller - I couldn't find DisableQueryClientEvaluation() function. 😢

[rowanmiller]

That was the name in RC1. If you are using our nightly builds, then the method is QueryClientEvaluationBehavior(...).

c# optionsBuilder .UseSqlServer(...) .QueryClientEvaluationBehavior(QueryClientEvaluationBehavior.Throw);

[gdoron]

@rowanmiller @divega Another (very good IMHO) argument for allowing/disallowing client side evaluation on per query level:

Lets say there is a query that I'm aware of it being (partially) locally evaluated and I'm OK with that, I don't want EF to raise warnings and "pollute" my logs with warnings I aware and OK with them.

Am I right?

[rowanmiller]

@gdoron yes that is the primary scenario for per-query overrides.

[francoisbeaussier]

Got bitten by this quite a few times in the last couple of months. I wish it had been off by default.

I'll try that setting on the optionsBuilder for now.

[gdoron]

@rowanmiller I can't find QueryClientEvaluationBehavior (still on RC2)

services.AddDbContext<ApplicationDbContext>(options =>
    options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection"), b => b.MigrationsAssembly("xxxxxxxx.Web")));

Am I missing a using statement or was it moved to somewhere else?

[rowanmiller]

Here is the code for RC2...

c# optionsBuilder.UseSqlServer( "<connection string>", options => options.QueryClientEvaluationBehavior(QueryClientEvaluationBehavior.Throw));

[skimedic]

My apologies if this has been covered elsewhere, but my bing-fu must be weak if it was. Now that we are on RTM, it appears that this has been removed/renamed. This issue [https://github.com/aspnet/EntityFramework/pull/5396] discusses removing the QueryClientEvaluationBehavior, but I'm just not clear on the replacement mechanism.

[ajcvickers]

@skimedic See here: https://docs.efproject.net/en/latest/querying/client-eval.html#disabling-client-evaluation

[skimedic]

@ajcvickers Thank you! My midi-chlorian count must indeed be low. I missed that several times.

[Tasteful]

If I turn off client evaluation in db context options, is there any method to explicit turn it on for a specific query?

[gdoron]

Unfortunately, currently there's none. 😭

[ajcvickers]

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.