Efcore: Please open the query translation pipeline for extension

[BalassaMarton]

Use cases

• Generating DB commands for operations not currently supported by EF (e.g. #795)
• Extending with provider-specific LINQ operators (like how linq2db allows a wide variety of SQL constructs)
• More flexible and robust query filters
• Query validation and transformation

The current query generation pipeline is hard to extend without relying on implementation details.

Proposal

Enable intercepting the translation steps in QueryCompilationContext.CreateQueryExecutor.
API details are to be discussed here, I don't have a concrete proposal yet.

[roji]

@BalassaMarton the query pipeline is already extensible in many different ways, thanks to the DI infrastructure of EF Core:

• Providers (and plugins) can easily add method and member translators
• Providers (and even users) can provide their own implementation of IRelationalSqlTranslatingExpressionVisitorFactory to translate other types of expressions
• Providers (and even users) can add query pipeline preprocessors (which run at the very beginning) and postprocessors (which run after translations). We do have plans to improve the API for inserting new preprocessing/postprocessing visitors for 5.0.

Admittedly we currently lack documentation on how to do the above - we definitely plan on making all this more discoverable for 5.0 (see https://github.com/aspnet/EntityFramework.Docs/issues/681 for example, as well as other issues).

Are there any specific scenarios you have in mind, where you're unsure how to proceed? Is your interest more as a provider writer, or as an end user?

[BalassaMarton]

I'm only interested as a user, but I'll look deeper into the visitors and try to understand how it can be extended, thank you.

[ajcvickers]

Note from team discussion: we need to document this (see https://github.com/aspnet/EntityFramework.Docs/issues/951 and https://github.com/aspnet/EntityFramework.Docs/issues/500) but following that we should look at adding sugar for common patterns to avoid the need to understand expression visitors for common custom translations.

[smitpatel]

@BalassaMarton & @powermetal63 - Can you provide details of exactly what you want to change in expression tree for which you need extensibility? We can determine based on that what could be right hook for it and if it is missing, add it. Query pipeline is a complex system which has multiple parts end to end. As @roji mentioned above, there are hooks a long the way depending what kind of task need to be done.

[powermetal63]

@smitpatel I would start with the ability to preprocess/transform the IQueryable.Expression before EF Core infrastructure. This will be very useful for lambda injection extensions like NeinLinq, LinqKit and similar which currently have to implement custom IQueryProvider and IQueryable just to do that. The end user is required to call a custom method like ToInjectable(), AsExpandanle(), which can easily be forgotten, and also custom query providers doesn't play well with EF Core queryable/async extensions.

Also it will be useful if we want to add "fixes" to some things you are not willing to (or have no time, or will do it in future version we cannot afford waiting for etc.), like the one here https://github.com/dotnet/efcore/issues/19930#issuecomment-587476008.

I guess all this falls into bullet #3 - Easy way to add query pipeline preprocessors. By easy I mean to be able to do that without implementing custom options in order to add services etc. Something as easy to use as the current ReplaceService.

[smitpatel]

Something as easy to use as the current ReplaceService.

That already works for everything you want to inject during compilation phase.

Anything before pre-funcletization can be done outside of EF before passing expression into EF.

[powermetal63]

@smitpatel But we don't want it do be done externally. I already gave you several examples. We don't want to ask the user to call custom methods like ToInjectable, AsExpandable, ConvertGroupJoins etc. in order get the functionality. Because we are extending/fixing EF/C# query limitations, and want these pure query expression transformations to be executed automatically for each EF query "compilation".

So what we want is an easy way to plug something similar to your current QueryTranslationPreprocessor.Process - like GroupJoinConverter from https://github.com/dotnet/efcore/issues/19930#issuecomment-587476008, or LinqKit Expand functionality. Or in other words, something similar to AddInterceptors, but for query preprocessors which then run before EF.

[smitpatel]

@powermetal63 - You can already replace QueryTranslationPreprocessor by using ReplaceService API on IQueryTranslationPreprocessorFactory service. Did you look at that functionality?

[powermetal63]

@powermetal63 - You can already replace QueryTranslationPreprocessor by using ReplaceService API on IQueryTranslationPreprocessorFactory service. Did you look at that functionality?

@smitpatel Sure I know it, but that's the problem - all I can do is to replace it. Let say I replace it and return custom QueryTranslationPreprocessor. Which one QueryTranslationPreprocessor should I inherit - QueryTranslationPreprocessor, RelationalQueryTranslationPreprocessor or something else - at that moment I have no idea which is the default (current) IQueryTranslationPreprocessorFactory, what dependencies it has and what type of QueryTranslationPreprocessor. it creates. And since EF relies on that query preprocessing, we can easily break it unintentionally.

Shortly, IMHO this factory service approach is quite limited - I'm not sure why you created it and what it will be used for internally by EF, but it definitely is not something I (or 3rd party extensions creators) want to dial with. We don't need to replace something existing, we need something additive like recently added interceptors.

---

P.S. Please don't get me wrong. I'm doing EF Core hacks/workarounds from the very beginning. Here we are discussing a good public way of extending EF Core. Cheers.

[smitpatel]

Here we are discussing a good public way of extending EF Core. Cheers.

Infrastructure to extend EF already exists as asked by OP. It is personal preference to use it or not. That has no affect on this issue whatsoever.

[powermetal63]

Here we are discussing a good public way of extending EF Core. Cheers.

Infrastructure to extend EF already exists as asked by OP. It is personal preference to use it or not. That has no affect on this issue whatsoever.

Where is the existing infrastructure you are talking about and how we as end users can use it? For instance, where and how I as end user can safely plug my GroupJoinConverter ?

[smitpatel]

If you are using SqlServer or Sqlite provider, then derive from RelationalQueryTranslationPreprocessor add your custom processor. Create a new implementation of IQueryTranslationPreprocessorFactory which during create method creates the new derived class you generated and use Replace service method to register your custom factory in EF Core provider and it works.

[powermetal63]

@smitpatel What if I'm using Npgsql? Oracle? What if someday SqlServer provider decides to use something like SqlServerQueryTranslationPreprocessor? Should I check every provider implementation and create derived factory/processor classes from theirs?

And doing all that in order to add a single line similar to

query = new GroupJoinFlatteningExpressionVisitor().Visit(query);

at the beginning? You have to be kidding.

Doing it publicly and easily is the essential part of/reason for OP request :

The current query generation pipeline is hard to extend without relying on implementation details.

[smitpatel]

It is public and as easy as calling replacing service with factory and adding your custom code.
If you want to dislike it, that is your choice.

[roji]

@powermetal63 you may be lacking some information (which is understandable as we unfortunately don't have much docs on this). As @smitpatel wrote, RelationalQueryTranslationProcessor is a public service which can be replaced, adding another visitor in a way that isn't provider-specific. In other words, this would work regardless of which relational provider is being used. All you have to do is override NormalizeQueryableMethod and run you visitor before everything else.

Try giving it a try - and let us know if there are any blockers.

[powermetal63]

@roji It might be public, but definitely NOT clean and NOT easy.

Again, all I need is to add a SINGLE line of code to let EF calls my single custom method before others.

Just compare the

optionsBuilder.AddInterceptors(new MyInterceptor());

to what you are suggesting here: create TWO additional custom classes (factory + processor), in order to call MyPreprocessor. The effort is pretty much the same as implementing custom query provider as many libraries currently do just to be able to preprocess the query expression before passing it to the original provider.

But even this is not the main problem. Apart from the fact that it's unclear which base class(es) to inherit with "replacements", the main problem is that this pattern is NOT additive. What if some library wants to add their preprocessor? If they take the path you are suggesting, then depending of the order of configuration calls either they will replace mine or vice versa. How about that?

Shortly, IQueryTranslationPreprocessorFactory / QueryTranslationPreprocessor can be used even currently for quick-and-dirty workarounds, but long term they do not provide the desired extensibility hook.

I'm ending the discussion here. I luckily don't use EF Core in my production software, so I have no "blockers" at all. All I wrote was as response to @smitpatel

Can you provide details of exactly what you want to change in expression tree for which you need extensibility?

I think I provided enough examples, including popular 3rd party packages showing the simplest extensibility hook needed to perform pure expression tree transformation. At this moment I have no opinion for other OP cases, but even if I do at some point, based on my experience with EF Core team discussions I don't think it worth my time arguing the obvious things.

[roji]

@powermetal63 yes, the existing solution does require you to write two additional trivial classes rather than a single line - for an advanced and niche scenario. As in #19930, we have almost no user requests for simplifying this further: people very rarely wish to integrate an expression tree visitor into the query pipeline. Although it may not fit with the specific idea you have of what you want, we consider the current solution clean and easy for the advanced scenario which it serves.

depending of the order of configuration calls either they will replace mine or vice versa. How about that?

~The same problem exists with your proposal of a simple AddInterceptors call.~ (NOTE: see below for clarification)

[powermetal63]

The same problem exists with your proposal of a simple AddInterceptors call.

@roji AddInterceptors was given as an example. If you don't know (?!), is part of an existing new interception API in EF Core 3.0. Are you saying that if some plugin calls AddInterceptors with it's own interceptors, and then another one calls it with theirs, the second will replace the previous? I don't think so. It is an example of "proper" extension point for db operations.

We want SIMILAR for IQueryable.Expression, why it is so hard to understand?? Following your style, all you need is to define new trivial single method interface and add few trivial lines of code in a few places.

Your current "solution" does not serve any even simple practical purpose. And the usage scenarios I've shown you are NOT advanced, because they solve query expression tree limitations YOU are not willing to solve (or have no resources/time which is understandable). Votes here doesn't matter, all these libraries - NeinLinq, LinqKit, DelegateDecompiler, and many others suffer from the lack of such extension point and have to take the custom IQueryProvider / IQueryable path, requiring calling special methods for EACH query, and if you don't know (?), NOT playing well with EF Core queryable and async extensions. Their authors simply don't go here. Which is understandable seeing this style of communication (basically, EF Core team are telling us what is good for us) and is just waste of time and energy.

Anyway, if you don't understand why your current "solution" doesn't work or isn't proper, there is nothing we can do.

[roji]

FWIW my comment above was imprecise - I didn't mean to suggest that AddInterceptors doesn't allow for adding multiple interceptors.

But our current API also allows that. A user may simply replace IQueryTranslationPreprocessorFactory in their application, calling multiple visitors coming from multiple libraries in the order they see fit. As such, there's nothing functionally lacking in our current API - you're only asking for a terser way to do the same, i.e. syntactic sugar.

And once again, from our long experience maintaining this project, people have simply not been asking for what you're asking, so we haven't prioritized anything like this. I'd guess that people simply aren't bothered with adding ToInjectable and the like, and if they are, they still have the option of replacing IQueryTranslationPreprocessorFactory.

[powermetal63]

people have simply not been asking for what you're asking

@roji If you look at the OP (which you even responded), you will see that I'm NOT the one who is asking for this. And your original reply to them was much more constructive than recently.

Anyway, I'm really taking off this and all EF Core discussions. As I said many times, I have absolutely no professional interest in using EF Core - I'm just your current FREE all time StackOverflow supporter, but that could easily be changed.

[ajcvickers]

I disagree with @roji and @smitpatel on this. As I said on Feb 3:

Note from team discussion: we need to document this (see dotnet/EntityFramework.Docs#951 and dotnet/EntityFramework.Docs#500) but following that we should look at adding sugar for common patterns to avoid the need to understand expression visitors for common custom translations.

I still believe that we should "look at adding sugar for common patterns to avoid the need to understand expression visitors for common custom translations" which was the conclusion we came to when discussing this with the team.

I'm not going to say any more about my thoughts now before discussing again with the whole team so that I fully understand the objections that are being raised by @smitpatel and @roji.

[Amigolden]

I agree with @powermetal63. He's provided multiple examples of why his approach would be better. It seems that @roji and @smitpatel's argument can be summed up as

  "We have and api that work and is easy"
  "people have simply not been asking for what you're asking"

However this is not the case.

When people have problems and Issues with the code there writing, They don't go and post a feature request on github.
They ask a question on Stackoverflow. If they don't get an answer they want, they'll do what easier at the time, which often can lead to using different technologies.

.Net-core is in its early stages, and you've been given an opportunity, to make the publicly exposed interface extendible and easy to use. @powermetal63 has been leading the forefront in addressing issues relating to .net-core. He of all people has a way better understanding of what people have been asking for.

The current implementation is overly verbose, it forces users to remember call ToInjectable() which is conding based on convention and error prone.

Please do the right thing for once and listen to the community.