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

Redo our pruning implementation #31083

Closed
Tracked by #31327
roji opened this issue Jun 15, 2023 · 0 comments · Fixed by #32672
Closed
Tracked by #31327

Redo our pruning implementation #31083

roji opened this issue Jun 15, 2023 · 0 comments · Fixed by #32672
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Jun 15, 2023

Our current pruning logic currently only operates on SelectExpressions which are present in the Tables list of other SelectExpressions; it notably doesn't handle subqueries embedded e.g. in the predicate.

For example, given the following entity type:

public class Blog
{
    public int Id { get; set; }
    public string? Name { get; set; }
    public int Unneeded { get; set; }
}

... and the following query:

_ = ctx.Blogs
    .OrderBy(b => b.Id)
    .Take(10)
    .Take(5) // Causes pushdown
    .Select(b => b.Name)
    .ToList();

... the Unneeded column gets properly pruned from the projection:

SELECT TOP(@__p_1) [t].[Name]
FROM (
    SELECT TOP(@__p_0) [b].[Id], [b].[Name]
    FROM [Blogs] AS [b]
    ORDER BY [b].[Id]
) AS [t]
ORDER BY [t].[Id]

But if you place the same thing inside a subquery:

_ = ctx.Blogs
    .Where(b => ctx.Blogs
        .OrderBy(b => b.Id)
        .Take(10)
        .Take(5) // Causes pushdown
        .Select(b => b.Name)
        .First() == "foo")
    .ToList();

... that yields the following SQL, which does project Unneeded:

SELECT [b].[Id], [b].[Name], [b].[Unneeded]
FROM [Blogs] AS [b]
WHERE (
    SELECT TOP(1) [t0].[Name]
    FROM (
        SELECT TOP(5) [t].[Name], [t].[Id]
        FROM (
            SELECT TOP(10) [b0].[Id], [b0].[Name], [b0].[Unneeded]
            FROM [Blogs] AS [b0]
            ORDER BY [b0].[Id]
        ) AS [t]
        ORDER BY [t].[Id]
    ) AS [t0]
    ORDER BY [t0].[Id]) = N'foo'

The reason for this is that the pruning logic isn't implemented as a proper visitor which visits the entire tree; instead, SelectExpressionPruningExpressionVisitor (in postprocessing) calls SelectExpression.Prune; this recurses into Tables, but not into the other parts of the query.

We should extract the pruning logic out to SelectExpressionPruningExpressionVisitor, taking care with SelectExpression private state etc. Note #31049 (comment) which is similar, abuot allowing providers to add other pruning logic.

@ajcvickers ajcvickers added this to the Backlog milestone Jul 7, 2023
roji added a commit to roji/efcore that referenced this issue Dec 25, 2023
roji added a commit to roji/efcore that referenced this issue Dec 25, 2023
roji added a commit to roji/efcore that referenced this issue Dec 25, 2023
roji added a commit to roji/efcore that referenced this issue Dec 25, 2023
@roji roji self-assigned this Dec 25, 2023
@roji roji modified the milestones: Backlog, 9.0.0 Dec 25, 2023
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 25, 2023
roji added a commit to roji/efcore that referenced this issue Dec 25, 2023
roji added a commit that referenced this issue Dec 29, 2023
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview1 Jan 31, 2024
@roji roji changed the title Prune all SelectExpressions, not just those in Tables Redo our pruning implementation Aug 23, 2024
@roji roji modified the milestones: 9.0.0-preview1, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants