Skip to content

Conversation

@kicken
Copy link
Contributor

@kicken kicken commented Mar 17, 2025

SQL Server does not allow ORDER BY clauses in sub-queries unless they are limited by a TOP / OFFSET clause which we don't want to do when getting an item count.

@kicken
Copy link
Contributor Author

kicken commented Mar 17, 2025

This was previous fixed in #150 but the fix was lost in #337

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the CS failure.
Also, can you ensure that the problem that lead to the previous change is not regressed?

@kicken
Copy link
Contributor Author

kicken commented Mar 18, 2025

I've removed the white space change.

I've added another test for the group by situation and it is passing. I believe the change should not cause and regression issues for that.

@kicken kicken requested a review from garak March 18, 2025 17:24
Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's OK for me. I'll merge as soon as you solve the CS issue

SQL Server does not allow ORDER BY clauses in sub-queries unless they are limited by a TOP / OFFSET clause which we don't want to do when getting an item count.
@garak garak merged commit eabf392 into KnpLabs:master Mar 20, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants