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

ActiveRecord count with a Relation#group returns a hash, not an integer, which break Pagination #984

Open
fractaledmind opened this issue Feb 20, 2017 · 4 comments

Comments

@fractaledmind
Copy link

I came across this bug in a large Rails API my team is building. The issue is that ActiveRecord count method does not always return an integer. As explained in the docs:

If count is used with Relation#group, it returns a Hash whose keys represent the aggregated column, and the values are the respective amounts:

Person.group(:city).count
# => { 'Rome' => 5, 'Paris' => 3 }

This breaks the expectation made in the various Paginator classes that record_count will be an integer.

The simple way to fix this is to handle the Hash case with hash.values.sum. I am putting in this Issue, instead of a MR, because I am not certain where in the stack you would think this should live. It could live in the AR adapter for find_count, it could live in the Processor when page_options[:record_count] is created, or it could live in the Paginator class.

If you give me any guidance on how you would like to fix this bug, I will happily put in the appropriate MR.

stephen

@kirlev
Copy link

kirlev commented Apr 3, 2017

@fractaledmind I encountered the same problem, I had to use a left outer join and group the results, the pagination breaks down because the 'count' returns a hash.
It is possible to just do hash.length but I would prefer that the counting was done by the database, it seems more right to me.
I found a related question on SO: http://stackoverflow.com/questions/12283847/how-can-i-count-the-result-of-an-activerecord-group-query-chaining-count-retu
What do you think about incorporating orourkedd's solution in the 'count_records' method inside 'lib/jsonapi/resource.rb' file?

@ercpereda
Copy link

ercpereda commented Oct 17, 2017

Thanks, @kirlev your comment helps me to build this workaround.

Person.group(:city).extending(GroupCountExtensions)
................
module GroupCountExtensions
  def count(*args)
    scope = except(:select).select("1")
    scope_sql = if scope.klass.connection.respond_to?(:unprepared_statement)
      scope.klass.connection.unprepared_statement { scope.to_sql }
    else
      scope.to_sql
    end
    query = "SELECT count(*) AS count_all FROM (#{scope_sql}) x"
    ActiveRecord::Base.connection.execute(query).first.try(:[], "count_all").to_i
  end
end

@joshuapinter
Copy link

What a sneaky bug. Never ran into this for years. My workaround was to just call:

Person.group(:city).to_a.size

But this isn't performant as it has to run the full query, return the entire results and then just count the objects in the resulting Array.

@ercpereda Looks like the proper way to keep it in SQL.

@bioform
Copy link

bioform commented Mar 2, 2020

For .count, I would say it is expected behavior.
But for .size it is an annoying bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants