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

Orderings when Offset is used #19031

Closed
cincuranet opened this issue Nov 22, 2019 · 5 comments · Fixed by #21187
Closed

Orderings when Offset is used #19031

cincuranet opened this issue Nov 22, 2019 · 5 comments · Fixed by #21187
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@cincuranet
Copy link
Contributor

When the query contains Offset, this code puts ORDER BY (SELECT 1) into the SQL. For the future it would make sense to have this part overridable instead of whole GenerateOrderings.

@ajcvickers ajcvickers added the good first issue This issue should be relatively straightforward to fix. label Nov 22, 2019
@ajcvickers ajcvickers added this to the 5.0.0 milestone Nov 22, 2019
@cincuranet
Copy link
Contributor Author

This could probably be handled using the #18999.

@takacsalbert
Copy link

When using with Distinct(), this leads to an invalid query.
This code:

context.Vocabularies.Where(x => x.Expression.StartsWith(startChar)).OrderBy(x => x.Expression).Select(x => x.Expression).Distinct().Skip(offset).Take(pageSize);`

Translates to this query:

SELECT DISTINCT [v].[Expression]
      FROM [Vocabulary] AS [v]
      WHERE (@__startChar_0 = N'') OR ([v].[Expression] IS NOT NULL AND (LEFT([v].[Expression], LEN(@__startChar_0)) = @__startChar_0))
      ORDER BY (SELECT 1)
      OFFSET @__p_1 ROWS FETCH NEXT @__p_2 ROWS ONLY

And the following SQL error occures

ORDER BY items must appear in the select list if SELECT DISTINCT is specified.

The problem is with ORDER BY (SELECT 1), it should be either ORDER BY 1 ORDER BY [v].[Expression]

I know, it's a strange combination, but sh!t happens. :)

@smitpatel
Copy link
Contributor

Distinct gives you unordered result set, doing Skip/Take on that without order by does not make sense. If we did fix that invalid SQL, it would likely be an error message asking to add OrderBy before Skip/Take.

@takacsalbert
Copy link

Distinct gives you unordered result set, doing Skip/Take on that without order by does not make sense. If we did fix that invalid SQL, it would likely be an error message asking to add OrderBy before Skip/Take.

Thanks for the reply.
Replacing the 'ORDER BY (SELECT 1)' with 'ORDER BY 1' making the query work. That's the only issue. Tried in SQL Management Studio.

@smitpatel
Copy link
Contributor

Order by 1 is ordering by first column in the result. It may work in this query, same may not be true for all the queries.

roji added a commit that referenced this issue Jun 9, 2020
@roji roji added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed good first issue This issue should be relatively straightforward to fix. labels Jun 9, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview7 Jun 22, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview7, 5.0.0 Nov 7, 2020
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. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants