Skip to content

Keep ordering in count subquery #1227

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

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Keep ordering in count subquery #1227

merged 2 commits into from
Sep 27, 2024

Conversation

aidanharan
Copy link
Contributor

@aidanharan aidanharan commented Sep 27, 2024

Keep the ordering in the count subquery.

Before PR the query generated by accounts.count(:firm_id) in CalculationsTest#test_order_should_apply_before_count was:

EXEC sp_executesql N'SELECT COUNT(count_column) FROM (SELECT [accounts].[firm_id] AS count_column FROM [accounts] ORDER BY [accounts].[id] ASC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY) subquery_for_count', N'@0 int', @0 = 4

It should be (DESC instead of ASC):

EXEC sp_executesql N'SELECT COUNT(count_column) FROM (SELECT [accounts].[firm_id] AS count_column FROM [accounts] ORDER BY [accounts].[id] DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY) subquery_for_count', N'@0 int', @0 = 4

Fixes test:

https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/actions/runs/11072699849/job/30767524773

  2) Failure:
CalculationsTest#test_order_should_apply_before_count [/usr/local/bundle/bundler/gems/rails-13d5f8720892/activerecord/test/cases/calculations_test.rb:261]:
Expected: 4
  Actual: 3

@aidanharan aidanharan changed the title Keep ordering if count subquery Keep ordering in count subquery Sep 27, 2024
@aidanharan aidanharan marked this pull request as ready for review September 27, 2024 19:21
@aidanharan aidanharan merged commit 78c2079 into main Sep 27, 2024
2 of 4 checks passed
@aidanharan aidanharan deleted the count_subquery_ordering branch September 27, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant