-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Returning SqlFragmentExpression
from HasTranslation
doesn't work (switches to client evaluation)
#32969
Comments
I have found a way to achieve the desired result but it is VERY hacky: public static void HasTotalRowCountFunction(this ModelBuilder mb)
{
mb
.HasDbFunction(() => TotalRowCount())
.HasTranslation(arguments =>
{
return new SqlFunctionExpression(
"COUNT",
new SqlExpression[] { new SqlFragmentExpression("*) OVER(") },
false,
new[] { false },
typeof(int),
null);
});
} But I am afraid that evil ghosts will haunt me in my nights dreams for the rest of my life if I put this into production... So I would really prefer a cleaner solution for this. |
Any chance you can post a minimal console program that shows this happening? |
Was trying to do the same by returing an SqlFragmentExpression directly but it will throw an InvalidOperationException: Returning a derived SqlExpression which then returns the SqlFragmentExpression works, but unfortunately not with subqueries. Reproduction code that tries to workaround the NULLS LAST issue in PostgreSQL in a very hacky way 😉
.csproj
|
@roji Here is a .NET Fiddle that shows the failing version and the dirty workaround hack. PS: I also tried the workaround shown in the post of @alienwareone (with a derived |
Problem 1: Can't use SqlFragment directly
Problem 2: Projection gets screwed up for subquery
|
@roji I too think that allowing
I hope this short API experience story is of some help. |
@InspiringCode thanks for the added details - that's useful indeed. FWIW scenarios where users need to construct their own SQL trees for functions are quite rare/advanced, and ones where they need some sort of SQL construct which isn't already supported is even rarer - we hardly ever see people doing that. In general, integrating arbitrary "stuff" into the SQL tree is more complicated than it looks - as you can see from the above; it's simply not possible to just "make it work" in all scenarios etc. But allowing a type mapping specifically may be OK - I'll discuss with the team. BTW I'd be interested in why you need to generate |
@roji Thank for taking the time to review the issue. My use case is the following:
public static async Task<PagedResult<T>> ToPagedResultAsync<T>(
this IQueryable<T> query,
int page,
int pageSize)
{
var result = await query
.Select(x => new
{
Item = x,
TotalCount = TotalRowCount()
})
.Skip(page * pageSize)
.Take(pageSize)
.ToArrayAsync();
return new PagedResult<T>(
Items: result.Select(x => x.Item).ToArray(),
Page: page,
TotalCount: result.FirstOrDefault()?.TotalCount ?? 0
);
} This was a lot faster than issuing two separate query (the additional row count for complex queries was barely noticeable). |
Thanks for the added detail - yeah, hopefully one day EF will have support for window functions (#12747), at which point such hacks will no longer be needed. Though another possibly better way to handle this would be to allow batching LINQ queries (#10879) - the downside with the window function hack is that at least in theory, the database is recalculating the total rows for each row it returns (though this is hopefully optimized away under the hood). |
@roji Query batching doesn't solve the problem in this case because the round-trip is not the expensive thing. Running I really hope EF will support window functions natively soon, because I think they are an essential tool in business applications. |
How is it possible that running COUNT(*) in a separate query would take more than |
@roji When using
Here is a comparison of the actual execution plan (above: with When I just run a normal |
Ah I see, the part I was missing was that the So thanks, that's a pretty convincing scenario for enabling some sort of support for window functions - I've added a note to #12747 (comment). |
Design decision: we think allowing SqlFragmentExpression to have a type mapping is reasonable; SqlFragmentExpression is a very advanced escape hatch, and it doesn't make much sense to specifically block that there (especially given that people are already working around that limitation as above). |
I've had the exact same experience with any expression not supported (in any minor way) by EF Core. This is a perfect example of why the whole There should be exactly one place where the SQL is generated: in the Basically what I'm saying is: |
@Charlieface I'd advise getting to know the innards of EF and what it means to generate SQL in-depth before making such statements. The problem with adding a new SQL expression is not QuerySqlGenerator at all; that's just a standard expression visitor that's trivial to extend, adding support for arbitrary new expression types. The problem is that SQL expression aren't just inserted into the tree and then printed - there's various intermediate processing around them. As an example, EF performs "type mapping inference"; that means that when you write Another example is nullability processing; EF performs various compensation to make the SQL behave in the same way around NULLs as the original C# code does (although C# has 2-value logic and SQL has 3-value logic). This requires EF to know about each different expression type, and process it correctly; take a look at SqlNullabilityProcessor to know more. In other words, the problem isn't QuerySqlGenerator, it's the fact that full support for a new expression type needs to be implemented in various stages of the EF query pipeline; all of these are extensible (at least to some extent), but the task isn't easy and requires in-depth knowledge of EF internals. That's just the reality of a complex, feature-rich ORM such as EF - it's easy to think you want to just add a SQL expression, but that's just not how it works. We've also seen very few people actually try to add expression types, and the right way to deal with that is to implement the feature that those users are looking for (e.g. window functions), rather than for users to try to insert support for custom SQL expressions into the ORM. |
I have the following function mapping with
HasTranslation
:But when I use the
TotalRowCount
function in a projection, theSqlFragmentExpression
is ignored and theTotalRowCount
function is evaluated client-side (theNotSupportedException
is thrown). When I return aSqlFunctionExpression
instead, the correct SQL is generated and the SQL function is correctly called.I am observing a similar behavior when using an `IMethodCallTranslator´ to do the same translation. But in this case another strange behavior can be observed: If my function has a parameter, the translator is called, if it has no parameters (provided by LINQ query), like the method above, the translator isn't even called.
I did look into the EF source code quite a bit but I am feeling kind of lost. Could you please give me a tip why this doesn't work, how I could possibly achieve this simple mapping or at least some hints where I could look in the source code.
I really appreciate any help. Thank you.
The text was updated successfully, but these errors were encountered: