Efcore: Injecting Expression Visitors in the dbcontext

[raffaeler]

This is not an issue but just a question.
Is it possible to inject my own Expression Visitors into the context so that they modify the user queries before EF goes on?
My current implementation wraps the dbcontext in my own linq provider that takes care of applying the visitors and finally execute the query in the wrapped dbcontext. But it would be far easier to have the ability to inject the visitors in the dbcontext itself.

[smitpatel]

https://github.com/aspnet/EntityFrameworkCore/blob/master/src/EFCore/Query/Internal/IQueryCompiler.cs

Derive from QueryCompiler (beware it's internal so can break in minor release) and override above methods to intercept Expression tree before they are executed by EF. Replace IQueryCompiler service with your derived class and it would be good to go.

[raffaeler]

@smitpatel Thank you.
Any chance to make it more 'official' so that it can be used in production? (my code is part of a framework that should be version-safe)

[smitpatel]

What kind of ExpressionVisitor you intend to use in production?

[raffaeler]

I have two different cases:

1. adding filters for authorization (I can't use the global filters as they do not support navigation properties)
2. fixing the navigation paths when I find a many-to-many (I built an infrastructure to avoid modifying the model with the intermediate entity)

Beyond those, I have other uses. For example it can be very useful to analyze the expressions during development mode to understand what is going to execute on the server side and what else on the client side.

[smitpatel]

@raffaeler - Do you need to process expression trees before query execution (which means the expression visitor will run for every query, cache or non-cached) or only while compiling (which means non-cache goes through the expression visitor but cached version can be used without the expression visitor)?

[raffaeler]

@smitpatel I have to process all the request going to the database, otherwise the filters or the access members won't be rewritten.

In the first case (filters) I would disclose informations the user can't see.
In the second case (member access to patch the many to many) the execution would cause an error.

[raffaeler]

@smitpatel given the current status, do you confirm that a custom linq provider wrapping EF is a better solution?

[smitpatel]

This would be the best way
https://github.com/aspnet/EntityFrameworkCore/issues/14528#issuecomment-458243577

[raffaeler]

@smitpatel got it.
With regards to your previous questions, I just wanted to be sure that visitors will be executed every time to avoid the problems I cited above. Are they invoked every time?
Thanks!

[smitpatel]

Yes. QueryCompiler is entry point for any query and wraps query caching inside of it. So if you run expression visitor before that, you will have it run before caching.

[raffaeler]

Thanks, tomorrow I will give it a try.

[raffaeler]

@ajcvickers I see you added this issue to the backlog.
Do you want me to reopen it?

[mrswain]

Would the change I propose in #14557 meet your needs? I use this to add custom support for null mapping so I can override the creating of the IsNullExpression. I am working on a way to handle extended value converters (I am trying to convert a numeric(8,0) yyyymmdd number to a Nullable, and properly handle the queries that ODATA creates.)

[raffaeler]

@mrswain I need to run one or more expression visitors before every database execution.
This can potentially rewrite the whole expression, without any limit (I did it in the past with EF6 where I splitted the query targeting half the model on a database and the other half on another database).

I am not sure what you want to expose with the plugin. The ability to override all the method calls in QueryCompiler is enough (I just completed my test few minutes ago, and it works). I just asked to expose this publicly so that it will be version-resistant :)

[mrswain]

The changes I proposed would add the ability to create a class that implements IExpressionFragmentTranslatorPlugin which in turn could provide one or more instances of classes that implement IExpressionFragmentTranslator.

The translator is responsible for receiving a portion of an expression tree and either return null; or whatever it may change the tree to. This is used by the SqlTranslatingExpressionVisitor as it walks the expression tree, which is done to translate a vanilla linq query into the expression tree that will be used by the DefaultQuerySqlGenerator to generate the actual SQL to execute. It would allow you to modify the expression tree prior to SQL generation, and those modifications would get cached and re-used for every query that uses the same tree.

currently the only way to inject your own IExpressionFragmentTranslators is to extend/replace the RelationalCompositeExpressionFragmentTranslator with a new one and use its protected methods to add the translators. My changes just add the support for plugins similar to what the MethodCall and Member translators already do.

[raffaeler]

@mrswain The fragment would not be sufficient for me to evaluate the meaning of the expression.
I always have to visit all the tree. For example, when you want to add a condition to a predicate:

• you override the visit of the call method and set a flag if the call method name is a where and the type matches with the desired one
• you override the binary expression and modify the node only if the flag of the call method is true
• if you have to modify the value specified in the predicate, you override the constant expression and modify it only if the flag of the call method is true.

This means that you cannot just evaluate the fragment of the binary expression because it must be changed only when you are inside the right predicate.
Other examples can be very different and requiring to observe other nodes.

In other words the scenario I have to address is different than yours.

[ajcvickers]

Note from team triage: while we do want to make it easier to add or override translations, given the way the query pipeline and the context are factored, this is not the the best approach.

[raffaeler]

@ajcvickers It would be nice to know if there is something I could do in the codebase now in order to prepare to the future approach.

My use-cases are already in this thread.

Thanks

[smitpatel]

@raffaeler - I read the thread for your use cases. Mainly https://github.com/dotnet/efcore/issues/14528#issuecomment-458275941

1. Global query filters allows navigations. Let us know if you are still blocked on that and need to do something custom for it. We expect that this should not require you to inject custom visitor.
2. M2M is going to be a lot more first class in 5.0. We have added ability for skip navigations so queries for M2M can be oblivious to join entity. We are also looking at making join entity definition not required as a stretch goal. See https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-5.0/plan#many-to-many-navigation-properties-aka-skip-navigations for more details.

For debugging purposes, we had added ToQueryString on IQueryables to inspect SQL being sent to server. Also if it is just debugging then it would be ok to use public classes which are in namespaces ending with .Internal. They could break between releases but it is dev time stuff so won't crash your app.

If after all above changes if you still need to inject visitor then you would need to provide details on the scenario. Based on what work is required there are quite a lot of different entry points to inject a visitor and there is no answer which fits all.

[raffaeler]

@smitpatel Those two scenarios were pretty good representing how we needed to manipulate the expressions, but there are definitely more.

To be clear, if there is no support for modifying the expressions, we will need to drop efcore entirely.

In a more complex scenario, we cannot control the queries as they come from different plugins.
For this reason, we have a semantic analysis leveraging the richness of the expression tree. The same would be too hard to do in pure sql as there are not enough semantic information.
The analysis is meant to understand how the query is going to impact the in-memory entities and is pretty complex but works very nicely.
It would be even better to have the ability to gracefully stop the query execution after analyzing the expression, instead of throwing.

Thanks

[smitpatel]

EF being open to provider at the least, is always going to have support for modifying the expressions. It is matter of what exact modification is and when it needs to be applied. Query translation pipeline has various phases and things need to happen in certain stage. We would like to work with customers to provide them guidance and show right way to do custom things hence details would really be useful.

[raffaeler]

@smitpatel sure, but if I have to write a provider to just intercept the expression, it would probably make more sense to adopt a different library that makes this stuff easier.
It is just a matter of the amount of work you have to invest to solve the problems. What I am asking is to leave some of the doors opened because there are use-cases tha may require it. In other words I hope not to have to struggle just because a key method is marked as "internal" as it happened for years in the .NET Framework.
Thanks.

[smitpatel]

I am not saying you have to write a provider to do anything customs. We have left a lot of doors open as public methods and not internal. I can list all the doors (or hooks we have to intercept expression tree processing) here for you but all it would do is to make it more difficult to understand which one to use. So all I am saying is, please let us know if anything specific task you want to do and we can guide you to which is the right way to do it. And if in the process, it turns out that it is not easy to override and intercept then we can create new hooks to intercept too.

[raffaeler]

Good to know the extensibility will stay there.
I just need to analyze the expression and eventually replace it. The analysis outcome will be used from the app to make speculations about the impacted entities.
Thanks