Efcore: Extension methods implementation weaknesses in EF Core 5

Created on 12 Nov 2020  路  7Comments  路  Source: dotnet/efcore

Hi,

Just started porting our extension to EF Core 5 and was really disappointed how Expression Tree is built.
There are few extension methods that I have found for now, but they inject internal Custom Expressions into expression tree even before execution.
It is DbSet<>, FromSqlRaw, FromSqlInterpolated and maybe more.

What is bad here: every third party library which are trying to simplify LINQ world, are affected because they have to know about these internal custom expressions QueryRootExpression, FromSqlQueryRootExpression. They should stick very close to EF Core implementation or use reflection to omit unwanted dependency to internal expressions which may change any time.

Which libraries it can impact:
Remote.Linq
Serialize.Linq
Almost every IQueryable caching libraries

My point is: it is not a big deal to add these methods to parsing logic, and as a positive moment, it will remove side effects to other common LINQ extensions.

closed-question customer-reported

Most helpful comment

None of the conclusions you are making is true. We are not forcing anyone to use internal API at all. Any custom expression being used in EF Core query pipeline which third party libraries need to intercept is in public API. If it is not public then it should not be intercepted. Especially the ones you specified above are query roots and a third party which is trying to remain agnostic to EF Core should not be intercepting those at all - method or custom expression - since base visitor will just walk over those nodes without modifying them. Now if a third party is intercepting them then they are tying to EF Core. It is no longer agnostic to EF Core as you claim.

I am all open to making it more public and extensible for third party as needed. But I am yet to see a scenario which is blocked in order for me to think how to provide extensibility hooks.
Further, 3rd parties you are linked, one of them does not work for ns2.1 so moot point and other one hasn't filed any issue to us ever. EF Core preview versions with aforementioned changes are already working for libraries like OData, dynamic linq , automapper.

All 7 comments

Please describe what exact issue you are hitting.
There is one thing common in all the methods you posted, they are query roots in EF Core.
Query roots has default visit children method already. If you library is not changing anything specific to EF then can ignore those expression altogether and it will still be fine. If your library is doing something with it then you are already trying a EF Core specific behavior.

I have found solution against this behavior, but I really don't like it.
EF Core starts doing unwanted things with Expression Tree like Relinq is doing correctness check in its IQueryProvider. Expected is a clean tree in IQueryable with methods invocation, not a custom internal expressions.

Expected is a clean tree in IQueryable with methods invocation, not a custom internal expressions.

That is fundamentally against design of expression trees. The reason there is ExpressionType.Extension that you can add custom expressions to the expression tree to do the processing. A method invocation is only correct if it is actual method call which does not have any associated meaning and special visiting need. If the expression represent something more than a method call or contains state than using method call is wrong design. The only place where custom expressions should be converted to basic expressions is when actually compiling it into delegate.

That being said, unless you tell us what specific issue you are facing, there is nothing actionable for us.

That being said, unless you tell us what specific issue you are facing, there is nothing actionable for us.

  1. You force third party libraries to use EF Core internal API.
  2. Serialization libraries has to use internal API. For them it is easiest to get full method name and it鈥檚 parameters.
  3. Caching libraries has to use internal API to create correct caching key.

I鈥檓 also good in LINQ parsing and i know what you are trying to simplify. But instead of making query a lazy as possible, there is forcing transformation and validation before execution.

Anyway it鈥檚 just my subjective opinion.

None of the conclusions you are making is true. We are not forcing anyone to use internal API at all. Any custom expression being used in EF Core query pipeline which third party libraries need to intercept is in public API. If it is not public then it should not be intercepted. Especially the ones you specified above are query roots and a third party which is trying to remain agnostic to EF Core should not be intercepting those at all - method or custom expression - since base visitor will just walk over those nodes without modifying them. Now if a third party is intercepting them then they are tying to EF Core. It is no longer agnostic to EF Core as you claim.

I am all open to making it more public and extensible for third party as needed. But I am yet to see a scenario which is blocked in order for me to think how to provide extensibility hooks.
Further, 3rd parties you are linked, one of them does not work for ns2.1 so moot point and other one hasn't filed any issue to us ever. EF Core preview versions with aforementioned changes are already working for libraries like OData, dynamic linq , automapper.

@sdanyliv what would help us understand your precise argument, is if you could point to a specific, concrete external library which fails because of EF Core's usage of custom expression nodes; or a simple code sample which illustrates what you are trying to do. As @smitpatel wrote, we suspect that the problem here is an incorrect approach to integrating with the query expression tree, rather than a problem in EF.

Closing, probably I have overestimated problem. Thanks for your patience.

Was this page helpful?
0 / 5 - 0 ratings