Skip to content
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

Translate Select() with index using ROW_NUMBER #24218

Open
Tracked by #25570
iPilot opened this issue Feb 22, 2021 · 2 comments
Open
Tracked by #25570

Translate Select() with index using ROW_NUMBER #24218

iPilot opened this issue Feb 22, 2021 · 2 comments

Comments

@iPilot
Copy link

iPilot commented Feb 22, 2021

Integrated with EF entities mapping from database based on row number

There is a simply way to translate IQueryable.Select((source, index) => ... ) into SQL using row_number() over ()
And optional actions based on query: SQL case operator or client side transformation.

Just an example

select * [, case when t1."ROW" = ... then ... end]
from (
	select *, row_number() over () as "ROW"
	from ... t0
	where ...
	order by t0."DATE" desc
	limit ...
) t1
@smitpatel
Copy link
Contributor

We currently do not support index based overloads for lambda. There are several cons to having support for it.

  • The entering element is purely client side thing to assign an element index value rather than something being correlated to server. While it is true that it can be mimicked to server using row_number function for most provider, but if the computation is as cheap as the client side equivalent remains question.
  • EF core can upto an extent be able to inject row_number to generate index value but the lambdas are not as is translated to server. Let's take a look at following query
context.Set<Customer>().Select(c => new { Customer = c, c.Address }).Where(c => c.Customer.Id == "ALFKI");
// which actually gets converted to following while translating
context.Set<Customer>().Where(c => c.Id == "ALFKI").Select(c => new { Customer = c, c.Address });

Above is useful since c.Address navigation will generate a join in database. Applying filter before the select allows us to generate join with one row on left side with address, compared to earlier version where we join Customer/Address tables fully before just selecting one customer.
If above optimization seems artificial then imagine having a client function over c.Address in projection. In that case, if we don't do Where movement then query will fail to translate since there is a Where after client eval.
What this movements depict is that lambda may get changed during translation (even before SQL comes into picture), to make more things translatable as long as results are same. The parameter in lambda as same shape as the result before the operator. When using index based lambdas, this movement of lambdas doesn't remain easy. It may work in some cases and fail in some cases. This causes many of the queries which are working right now, not work in case of index based overloads (regardless of how the index is being used).

  • Because of above limitations, the best scenario where index based would work just fine would be very last Select method. At which point the value of index is the index in the result set. In most cases that could be done on client side and used in results by rewriting query appropriately.
  • There are also intricacies around row_number being non-deterministic/tie-breaker rules/partitioning, which doesn't exist for client side indexers in lambda.

Overall, while there seem to be translation existing the usability of it and value it provides is very minimal, not to mention the high cost it incurs because it is not natural translation for index in SQL. Hence we decided not to support it.

We have a tracking item in backlog to provide support for window functions in queries #12747 It will allow you to generate the query the way you want without the cost we translating the indexer in that form implicitly, giving more control over execution and avoiding issues about lambda movements.

@ajcvickers ajcvickers added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed type-enhancement labels Feb 23, 2021
@smitpatel smitpatel removed their assignment Jan 12, 2022
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@roji roji changed the title IQueryable Select with index overload support Translate Select() with index using ROW_NUMBER Feb 27, 2024
@roji
Copy link
Member

roji commented Feb 27, 2024

Reopening to reconsider this.

For perf reasons, we are now considering doing away with the pending selector logic which moves Select() forward (as part of #32957), which would make at least some of the problems above go away.

Thanks for @virzak for re-raising this in #12747 (comment).

@roji roji reopened this Feb 27, 2024
@roji roji added type-enhancement area-query and removed closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. labels Feb 27, 2024
@ajcvickers ajcvickers added this to the Backlog milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants