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

Query: Simplify aliases for complex projection if being used in order by clause #6703

Closed
smitpatel opened this issue Oct 6, 2016 · 6 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@smitpatel
Copy link
Contributor

For query like this

selection = db.Nodes.Include(x => x.NodeRecords)
        .Select(s => new HighNode()
        {
            Name = s.Name,
            Change = s.NodeRecords.OrderByDescending(o => o.NodeRecordId).Skip(days).FirstOrDefault().Change
        }).OrderByDescending(t => t.Change).Take(100).ToList();

We generate Sql like this

SELECT TOP(@__p_1) [x].[Name], (
    SELECT [o0].[Change]
    FROM [NodeRecords] AS [o0]
    WHERE [x].[Name] = [o0].[Node_Name]
    ORDER BY [o0].[NodeRecordId] DESC
    OFFSET @__days_0 ROWS FETCH NEXT 1 ROWS ONLY
)
FROM [Nodes] AS [x]
ORDER BY (
    SELECT [o].[Change]
    FROM [NodeRecords] AS [o]
    WHERE [x].[Name] = [o].[Node_Name]
    ORDER BY [o].[NodeRecordId] DESC
    OFFSET @__days_0 ROWS FETCH NEXT 1 ROWS ONLY
) DESC

We can just alias the subquery in projection and use alias in order by clause as enhancement.

For model and more info see #6257

@rowanmiller rowanmiller added this to the 1.2.0 milestone Oct 7, 2016
@smitpatel
Copy link
Contributor Author

Another expression to alias is COALESCE

@smitpatel
Copy link
Contributor Author

smitpatel commented Dec 6, 2016

This is a necessity for the case of coalesce when it is inside a subquery because we need to copy order by on outer query but the columns for coalesce are not projected out hence generates invalid query like this.
Disabling test Select_take_skip_null_coalesce_operator

exec sp_executesql N'SELECT [t].*
FROM (
    SELECT TOP(@__p_0) [c].[CustomerID], [c].[CompanyName], COALESCE([c].[Region], N''ZZ'') AS [c0]
    FROM [Customers] AS [c]
    ORDER BY COALESCE([c].[Region], N''ZZ'')
) AS [t]
ORDER BY COALESCE([t].[Region], N''ZZ'')
OFFSET @__p_1 ROWS',N'@__p_0 int,@__p_1 int',@__p_0=10,@__p_1=5

@smitpatel
Copy link
Contributor Author

@anpete @maumar - I was looking this in my current work. For complex expressions, we are able to alias the projection and use it in order by but for subquery it is failing because we generate different select expression (essentially the table aliases are different) because our subquerymodelvisitor will run twice on same qm. Thoughts where we should optimize this?

@smitpatel
Copy link
Contributor Author

@anpete @maumar - For all cases except subquery, alias-ing of ordering should be working. As explained above, to alias select expression in order by we need that subqueryExpression referencing same QM generating same SelectExpression. Do we want to keep this issue open now? Or merge into the larger issue?

@divega
Copy link
Contributor

divega commented May 8, 2017

Do we want to keep this issue open now? Or merge into the larger issue?

@smitpatel this got closed automatically when you pushed the commit, that might explain why no one answered 😄 Should it be reopened? Otherwise closed as fixed?

@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 8, 2017
@smitpatel smitpatel changed the title Query: Alias subquery if being used in order by clause Query: Alias complex projection if being used in order by clause May 8, 2017
@smitpatel
Copy link
Contributor Author

We do alias complex projections appearing in projection & order by both and refer to the alias in order by clause.
For subqueries, the generated select expressions are different so we have no way to match them and use alias.
All possible work for this issue has been done at this point.

@ajcvickers ajcvickers changed the title Query: Alias complex projection if being used in order by clause Query: Simplify aliases for complex projection if being used in order by clause May 9, 2017
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

4 participants