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

Changing how the select_count method counts returned rows. #687

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

jwoertink
Copy link
Member

Fixes #686

This new way now allows for counting against more complex queries which include using distinct_on. The main reason for this is using pagination when you have these complex queries.

Before this PR, you couldn't do this:

get "/messages" do
  pages, messages = paginate(MessageQuery.new.distinct_on(&.status))
  html IndexPage, messages: messages, pages: pages
end

This would throw an error that status needs to be in a "GROUP BY" or used in an aggregate function. But of course, if you added status to a group, it would also want id, and you'd still have to change the count from COUNT(*) to COUNT(status) or something else...

Also as @wyhaines pointed out in 686, there's several cases where this method is actually faster than the previous. All of the queries I tried seemed to perform about the same speed if not faster.

Comment on lines +199 to +200
table = "(#{query.statement}) AS temp"
new_query = Avram::QueryBuilder.new(table).select_count
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be cleared up if we implemented #682

Comment on lines -768 to -771
it "raises when used with offset or limit" do
expect_raises(Avram::UnsupportedQueryError) do
UserQuery.new.limit(1).select_count
end
Copy link
Member Author

Choose a reason for hiding this comment

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

You can now use a limit and offset in the select_count queries. The reason you couldn't before was because SELECT COUNT() would always ever return 1 row. So it didn't make sense to have a limit or offset. Now with this sub-select way, you can use limit to your advantage.

Say you have a table with 7 records in it. If you do a limit 10, and then run a count, you'll get 7. Do the same thing once there's 12 records in the table, and then you'll only get 10. It's sort of like an upper bounds check.

Copy link
Contributor

@robacarp robacarp left a comment

Choose a reason for hiding this comment

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

I like this. I run into this problem in Postgres a lot.

@jwoertink jwoertink merged commit 4971320 into master Jun 17, 2021
@jwoertink jwoertink deleted the issues/686 branch June 17, 2021 20:02
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.

Changing how COUNT() works in queries
3 participants