adding the pagination_count option for index page #2283 #2333

joseluistorres · 2013-07-10T16:59:30Z

@daxter Please provide feedback, I still don't know if I create enough tests.

coveralls · 2013-07-10T17:20:33Z

Coverage increased (+0%) when pulling a7b387a38c1a0b7e9c60127e60d0956814e9ddd7 on joseluistorres:issue_with_count_for_big_tables_2283 into ec99964 on gregbell:master.

seanlinsley · 2013-07-10T20:02:59Z

features/index/pagination.feature

+      Given 100 posts exist
+      When I am on the index page for posts
+      Then I should see pagination with 2 pages
+      And I should not see "Displaying Posts 1 - 10 of 100 in total"


You should add an extra constraint that also checks the expected string:

Then I should see "Displaying Posts 1 - 10" And I should not see "Displaying Posts 1 - 10 of 100 in total"

seanlinsley · 2013-07-10T20:34:54Z

The reason why your code was still causing the SELECT COUNT(*) query to occur was because the variable that holds the value was still being assigned, even if the string output wasn't using it.

I did a bit of a rewrite to achieve this:

if collection.num_pages < 2
  # ...
else
  offset = (collection.current_page - 1) * collection.limit_value
  if @display_total
    total  = collection.total_count
    I18n.t 'active_admin.pagination.multiple', :model => entries_name, :total => total,
      :from => offset + 1, :to => offset + collection_size
  else
    I18n.t 'active_admin.pagination.multiple_without_count', :model => entries_name,
      :from => offset + 1, :to => offset + collection_size
  end
end

With @display_total being defined in build:

def build(collection, options = {})
  @collection     = collection
  @param_name     = options.delete(:param_name)
  @download_links = options.delete(:download_links)
  @display_total  = options.delete(:pagination_count) { true }

seanlinsley · 2013-07-10T20:39:28Z

Now that I really think about it, it might be better to use multiple_without_total for the I18n key instead of multiple_without_count.

As well, the actual Ruby option might be better named as :pagination_total.

joseluistorres · 2013-07-11T15:41:18Z

@daxter branch updated...

coveralls · 2013-07-11T16:00:39Z

Coverage increased (+0%) when pulling 6aba6a4f4b3087a353ecffb8398a09ffa02d210c on joseluistorres:issue_with_count_for_big_tables_2283 into ec99964 on gregbell:master.

seanlinsley · 2013-07-15T17:33:55Z

features/index/pagination.feature

+      When I am on the index page for posts
+      Then I should see pagination with 2 pages
+      Then I should see "Displaying Posts 1 - 30"
+      And I should not see "Displaying Posts 1 - 10 of 100 in total"


Why does one say 1 - 30 while the other says 1 - 10?

Also, could you add a newline to this file?

My bad, it was a typo

joseluistorres · 2013-07-16T15:35:48Z

@daxter not sure what's wrong with the build. I bunch of failures from activeadmin.comments:

1) Comments ActiveAdmin::Comment Associations and Validations

Failure/Error: subject { ActiveAdmin::Comment.new }

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:7:in `new'

# ./spec/unit/comments_spec.rb:7:in `block (3 levels) in <top (required)>'

# ./spec/unit/comments_spec.rb:10:in `block (4 levels) in <top (required)>'

2) Comments ActiveAdmin::Comment Associations and Validations

Failure/Error: subject { ActiveAdmin::Comment.new }

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:7:in `new'

# ./spec/unit/comments_spec.rb:7:in `block (3 levels) in <top (required)>'

# ./spec/unit/comments_spec.rb:11:in `block (4 levels) in <top (required)>'

3) Comments ActiveAdmin::Comment Associations and Validations

Failure/Error: subject { ActiveAdmin::Comment.new }

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:7:in `new'

# ./spec/unit/comments_spec.rb:7:in `block (3 levels) in <top (required)>'

# ./spec/unit/comments_spec.rb:12:in `block (4 levels) in <top (required)>'

4) Comments ActiveAdmin::Comment Associations and Validations

Failure/Error: subject { ActiveAdmin::Comment.new }

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:7:in `new'

# ./spec/unit/comments_spec.rb:7:in `block (3 levels) in <top (required)>'

# ./spec/unit/comments_spec.rb:13:in `block (4 levels) in <top (required)>'

5) Comments ActiveAdmin::Comment Associations and Validations

Failure/Error: subject { ActiveAdmin::Comment.new }

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:7:in `new'

# ./spec/unit/comments_spec.rb:7:in `block (3 levels) in <top (required)>'

# ./spec/unit/comments_spec.rb:14:in `block (4 levels) in <top (required)>'

6) Comments ActiveAdmin::Comment.find_for_resource_in_namespace should return a comment for the resource in the same namespace

Failure/Error: @comment = ActiveAdmin::Comment.create! :resource => post,

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:22:in `block (4 levels) in <top (required)>'

7) Comments ActiveAdmin::Comment.find_for_resource_in_namespace should not return a comment for the same resource in a different namespace

Failure/Error: @comment = ActiveAdmin::Comment.create! :resource => post,

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:22:in `block (4 levels) in <top (required)>'

8) Comments ActiveAdmin::Comment.find_for_resource_in_namespace should not return a comment for a different resource

Failure/Error: @comment = ActiveAdmin::Comment.create! :resource => post,

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:22:in `block (4 levels) in <top (required)>'

9) Comments ActiveAdmin::Comment.resource_id_cast should cast resource_id as string

Failure/Error: comment = ActiveAdmin::Comment.create! :resource => post,

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:46:in `block (4 levels) in <top (required)>'

10) Comments ActiveAdmin::Comment.resource_id_type should be :string

Failure/Error: ActiveAdmin::Comment.resource_id_type.should eql :string

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./lib/active_admin/comments/comment.rb:37:in `resource_id_type'

# ./spec/unit/comments_spec.rb:55:in `block (4 levels) in <top (required)>'

11) Comments ActiveAdmin::Comment Commenting on resource with string id should allow commenting

Failure/Error: comment = ActiveAdmin::Comment.create! :resource => tag,

ActiveRecord::StatementInvalid:

Could not find table 'active_admin_comments'

# ./spec/unit/comments_spec.rb:64:in `block (4 levels) in <top (required)>'

12) ActiveAdmin::Filters::ViewHelper belongs_to when polymorphic relationship should raise an error if a collection isn't provided

Failure/Error: expect {

expected Formtastic::PolymorphicInputWithoutCollectionError, got #<ActiveRecord::StatementInvalid: Could not find table 'active_admin_comments'>

# ./spec/unit/filters/filter_form_builder_spec.rb:293:in `block (4 levels) in <top (required)>'

13) ActiveAdmin::FormBuilder when polymorphic relationship should raise error

Failure/Error: lambda {

expected Formtastic::PolymorphicInputWithoutCollectionError, got #<ActiveRecord::StatementInvalid: Could not find table 'active_admin_comments'>

# ./spec/unit/form_builder_spec.rb:74:in `block (3 levels) in <top (required)>'

Finished in 29.4 seconds

844 examples, 13 failures, 12 pending

joseluistorres · 2013-07-19T18:52:10Z

Hi @daxter, do you know if CI is working properly? I still don't know why are those failed tests not happening in my local. Perhaps related to #2351 ?

seanlinsley · 2013-08-01T20:57:50Z

Sorry, I was on vacation. Can you try rebasing this on master to see if that fixes these tests?

seanlinsley · 2013-08-01T21:39:08Z

Cool. One more thing, though. Can you add an example to the docs in the Index Pagination section?

Something like this would be great:

If you have a very large database, you might want to disable SELECT COUNT(*) queries caused by the pagination info at the bottom of the page:
index :pagination_total => false do
  # ...
end

seanlinsley · 2013-08-02T16:44:30Z

You added to the bottom of the file, but could you put it inside the "Index Pagination" section?

joseluistorres · 2013-08-02T16:48:40Z

LOL good point, I guess I never read that DOC before :)

seanlinsley · 2013-08-02T17:18:22Z

Once you get that updated, I'll gladly merge this.

coveralls · 2013-08-02T17:58:24Z

Coverage increased (+0%) when pulling 15fc073 on joseluistorres:issue_with_count_for_big_tables_2283 into 3cbaaaa on gregbell:master.

…_tables_2283 adding the pagination_count option for index page #2283

seanlinsley · 2013-08-02T18:07:25Z

Thanks for the contribution!

Fivell · 2013-10-31T19:30:53Z

Sorry I don't understand , kaminari still use count according to logs, this do nothing except of hiding displaying of total count in view. Manual says that it should disable SELECT COUNT(*) queries

seanlinsley · 2013-10-31T19:52:44Z

Perhaps what you're seeing is caused by scopes you have on your index page?

Fivell · 2013-10-31T20:03:06Z

no, I have no scopes.

ActiveAdmin.register IpAddress do
  belongs_to :company, :optional => true

  index :pagination_total => false do
    column :company
    column :ip
    column :date_time , :sortable => :updated_at do |row|
      row.updated_at
    end
    default_actions
  end

  controller do
     def scoped_collection
       super.includes(:company)
     end

     def permitted_params
       params.permit ip_address: [:ip, :company_id]
     end
   end
end

Processing by Admin::IpAddressesController#index as HTML
  AdminUser Load (4.1ms)  SELECT "admin_users".* FROM "admin_users" WHERE "admin_users"."id" = 1 ORDER BY "admin_users"."id" ASC LIMIT 1
  Company Load (0.9ms)  SELECT "companies".* FROM "companies"
   (4.7ms)  SELECT COUNT(count_column) FROM (SELECT 1 AS count_column FROM "ip_addresses" WHERE ('t'='t') LIMIT 30 OFFSET 0) subquery_for_count
  CACHE (0.0ms)  SELECT COUNT(count_column) FROM (SELECT 1 AS count_column FROM "ip_addresses" WHERE ('t'='t') LIMIT 30 OFFSET 0) subquery_for_count
   (1.0ms)  SELECT COUNT(*) FROM "ip_addresses" WHERE ('t'='t')
  CACHE (0.0ms)  SELECT COUNT(count_column) FROM (SELECT 1 AS count_column FROM "ip_addresses" WHERE ('t'='t') LIMIT 30 OFFSET 0) subquery_for_count
  IpAddress Load (1.0ms)  SELECT "ip_addresses".* FROM "ip_addresses" WHERE ('t'='t') ORDER BY "ip_addresses"."id" desc LIMIT 30 OFFSET 0
  Company Load (0.9ms)  SELECT "companies".* FROM "companies" WHERE "companies"."id" IN (1)
  Rendered /Users/admin/.rvm/gems/ruby-1.9.3-p125/bundler/gems/active_admin-9f6b03030667/app/views/active_admin/resource/index.html.arb (414.4ms)
Completed 200 OK in 432ms (Views: 410.2ms | ActiveRecord: 12.5ms)

Fix pagination_total index option to prevent SELECT count(*) queries close #2638 refs #2333, #2283

seanlinsley reviewed Jul 10, 2013
View reviewed changes

seanlinsley reviewed Jul 15, 2013
View reviewed changes

adding the pagination_count option for index page #2283

15fc073

seanlinsley added a commit that referenced this pull request Aug 2, 2013

Merge pull request #2333 from joseluistorres/issue_with_count_for_big…

779d009

…_tables_2283 adding the pagination_count option for index page #2283

seanlinsley merged commit 779d009 into activeadmin:master Aug 2, 2013

seanlinsley mentioned this pull request Aug 2, 2013

pagination uses COUNT(*) which doesn't work for large databases #2283

Closed

seanlinsley mentioned this pull request Oct 31, 2013

We still aren't able to prevent catch-all COUNT queries for index pages #2638

Closed

timoschilling added a commit that referenced this pull request Apr 11, 2015

Merge pull request #3848 from losvedir/3847-fix-pagination-total-false

3f9efd1

Fix pagination_total index option to prevent SELECT count(*) queries close #2638 refs #2333, #2283

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding the pagination_count option for index page #2283 #2333

adding the pagination_count option for index page #2283 #2333

joseluistorres commented Jul 10, 2013

coveralls commented Jul 10, 2013

seanlinsley Jul 10, 2013

seanlinsley commented Jul 10, 2013

seanlinsley commented Jul 10, 2013

joseluistorres commented Jul 11, 2013

coveralls commented Jul 11, 2013

seanlinsley Jul 15, 2013

joseluistorres Jul 15, 2013

joseluistorres commented Jul 16, 2013

joseluistorres commented Jul 19, 2013

seanlinsley commented Aug 1, 2013

seanlinsley commented Aug 1, 2013

seanlinsley commented Aug 2, 2013

joseluistorres commented Aug 2, 2013

seanlinsley commented Aug 2, 2013

coveralls commented Aug 2, 2013

seanlinsley commented Aug 2, 2013

Fivell commented Oct 31, 2013

seanlinsley commented Oct 31, 2013

Fivell commented Oct 31, 2013

adding the pagination_count option for index page #2283 #2333

adding the pagination_count option for index page #2283 #2333

Conversation

joseluistorres commented Jul 10, 2013

coveralls commented Jul 10, 2013

seanlinsley Jul 10, 2013

Choose a reason for hiding this comment

seanlinsley commented Jul 10, 2013

seanlinsley commented Jul 10, 2013

joseluistorres commented Jul 11, 2013

coveralls commented Jul 11, 2013

seanlinsley Jul 15, 2013

Choose a reason for hiding this comment

joseluistorres Jul 15, 2013

Choose a reason for hiding this comment

joseluistorres commented Jul 16, 2013

joseluistorres commented Jul 19, 2013

seanlinsley commented Aug 1, 2013

seanlinsley commented Aug 1, 2013

seanlinsley commented Aug 2, 2013

joseluistorres commented Aug 2, 2013

seanlinsley commented Aug 2, 2013

coveralls commented Aug 2, 2013

seanlinsley commented Aug 2, 2013

Fivell commented Oct 31, 2013

seanlinsley commented Oct 31, 2013

Fivell commented Oct 31, 2013