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

Pagination without having to SELECT COUNT(*). #681

Closed
wants to merge 1 commit into from

Conversation

bryanrite
Copy link

This PR is unfinished (no specs), I just wanted to test the waters as to whether you would want it.

This is a tweak I've had to use on several projects so that Kaminari doesn't do a COUNT due to slow Postgres COUNT issues, as also described: #240, #545. I cannot use indexes due to where conditions, or any other fancy ways to get the count as the tables are just too big, so this PR adds the ability to use paginate_without_count to paginate using next and previous links that never call COUNT.

Instead of COUNTing the dataset, we assume that if the current paged set is equal to the limit value, there is another page. We only expose next and previous links, since we have no way of knowing the number of pages. We override the Pagination class simply so we can change the partial class. Making the partial class configurable internally would alleviate that override if that is cleaner.

The only issue is if your count is exactly divisible by the per page limit, we'll show an erroneous next link on the last page... but to me this is a small price to pay for being able to page without timing out due to PG's sequential COUNT scan.

Thoughts?

@bryanrite bryanrite changed the title Pagination without having to SELET COUNT(*). Pagination without having to SELECT COUNT(*). Apr 14, 2015
@bryanrite
Copy link
Author

This build is failing because ActiveRecord edge is Rails 5 which requires Ruby > 2.2.1

@iwanwan
Copy link

iwanwan commented May 22, 2015

I support pushing this along... we have to do this on basically all of my projects when they reach a certain size. It would be great to have a built-in solution to this problem. Perhaps a :count => false option on paginate or something... but same idea.

@dogweather
Copy link

👍

@bryanrite
Copy link
Author

@amatsuda anything holding this up? thoughts?

@dogweather
Copy link

@yuki24
Copy link
Member

yuki24 commented Jun 21, 2015

@bryanrite As you mentioned above, if the count is exactly divisible by the per page limit, it'll show an erroneous next link on the last page. We can't accept this as I'm pretty sure someone will complain later on. I personally don't like it either.

We know there's a problem, but we don't accept fragile solutions.

@bryanrite
Copy link
Author

@dogweather Thanks, thats a great solution, but you still need to know the page count. The situation I'm building this for is when data is huge and changing too constantly for too many conditionals. Here is a contrived example:

Think of a user's activity feed on a site that has millions of users. I could keep a cache of the number of activities, which is fine for the main page, but then I want to show them only certain types of activities on another page. Now I have to cache that count as well, etc. etc.

The actual count of the activities isn't important. Allowing them to just "See More" is basically what I'm getting at, and Kaminari doesn't allow you to do that without a COUNT.

@yuki24 I know its an issue, but if you're using this method, your dataset is obviously quite big and reaching the end of such a huge dataset and being exactly divisible is likely uncommon. For something like an ajax infinitely scrolling list its a non-issue and/or easily handled in the UI (I show a "You've Reached the End" type of entry).

If you think that is serious enough not to include, i'll just gemify this.

@bryanrite
Copy link
Author

Thanks for the feed back 😄

@yuki24
Copy link
Member

yuki24 commented Jun 21, 2015

@bryanrite go ahead and gemify it.

@bryanrite
Copy link
Author

What if the PaginatorWithoutCount class used limit + 1 to make the query, but only output limit. It would act exactly the same but we would know if there was at least one more record, so we wouldn't erroneously show a "next" link on the last page.

@yuki24
Copy link
Member

yuki24 commented Nov 2, 2015

@bryanrite I like the idea of using limit + 1. Could you rebase and update this PR so we can talk further more?

@yuki24
Copy link
Member

yuki24 commented Nov 2, 2015

@bricker Supporting multiple ORMs might be hard, but let's not worry about it for now and focus only on AR.

@contentfree
Copy link

@bryanrite Think you can incorporate the limit + 1 change and update the PR?

@bryanrite
Copy link
Author

@contentfree Yup, I'm full up until the end of this month, but I'll take a look over the holidays and hopefully get this pushed.

@shir
Copy link

shir commented Jan 18, 2016

Hi @bryanrite
Looks like you missed require for new helper. When I use paginate_without_count I have error ActionView::Template::Error (uninitialized constant Kaminari::Helpers::PaginatorWithoutCount).

I added it:

--- a/lib/kaminari.rb
+++ b/lib/kaminari.rb
@@ -29,6 +29,7 @@ require 'kaminari/config'
 require 'kaminari/exceptions'
 require 'kaminari/helpers/action_view_extension'
 require 'kaminari/helpers/paginator'
+require 'kaminari/helpers/paginator_without_count'
 require 'kaminari/models/page_scope_methods'
 require 'kaminari/models/configuration_methods'
 require 'kaminari/hooks'

And now it works as expected.

@contentfree
Copy link

@bryanrite Were you able to update the PR with the limit + 1 change (and incorporate @shir's fix)?

@bryanrite
Copy link
Author

Bah, I completely forgot about this. Let me take a look again.

@amatsuda
Copy link
Member

amatsuda commented Apr 1, 2016

@bryanrite Please do! I really like the limit + 1 approach, and would love to have it in the core :)

@contentfree
Copy link

@bryanrite : ping! Could you update the PR and incorporate @shir's fix?

@stephenhyde
Copy link

stephenhyde commented Aug 15, 2016

@bryanrite : I am very interested in this PR, can we review and merge?

@yuki24
Copy link
Member

yuki24 commented Nov 7, 2016

Pagination without count is now available in the pagination-without-count branch. Give it a shot!

Gemfile:

gem 'kaminari', github: 'amatsuda/kaminari', ref: 'pagination-without-count'

In a controller:

@users = User.page(params[:page]).without_count

In a view:

<%= paginate_without_count @users %>

@PikachuEXE
Copy link

@yuki24
What's the difference between paginate and paginate_without_count?

@yuki24
Copy link
Member

yuki24 commented Nov 7, 2016

@PikachuEXE Well, It's a lot different from #paginate, but:

  • From a user's perspective: Users can only see links to previous or next page
  • From a developer's perspective: It avoids calling count by retrieving limit + 1(thanks to @bryanrite for the idea) if the scope is generated with the #without_count method
  • From a kaminari contributor's perspective: Have a look the diff

@cgriego
Copy link

cgriego commented Nov 7, 2016

It's going to be awesome to have this feature!

I wonder if the naming could be better. Saying "without count" is an implementation detail, even if it is an important one is some cases, but it isn't always important and it is one lost on novice developers. Maybe it would be clearer and lead developers to using the better option by default in more cases if we instead had paginate_numbered and paginate_next_prev?

@yuki24
Copy link
Member

yuki24 commented Nov 14, 2016

@cgriego Totally! Actually, you don't have to use #paginate_without_count. Instead, you can just use #link_to_prev_page and #link_to_next_page with a "without count" scope and they should work fine. Also, the PaginatorWithoutCount class seems overengineering to me. Maybe we can just get rid of #paginate_without_count entirely?

amatsuda added a commit that referenced this pull request Dec 3, 2016
based on @bryanrite's idea on #681 and @yuki24's experimental implementation
@yuki24 yuki24 added this to the 1.0.0 milestone Dec 3, 2016
@yuki24 yuki24 self-assigned this Dec 3, 2016
yuki24 pushed a commit that referenced this pull request Dec 5, 2016
based on @bryanrite's idea on #681 and @yuki24's experimental implementation
@yuki24 yuki24 closed this in 48ba1f0 Dec 5, 2016
@schneems
Copy link

Is it possible to use the regular paginate widget and pass in count? I have a counter cache on the model so the query does not need to be executed on every page load. Is there an option that works to pass in count, or is it only this?

@yuki24
Copy link
Member

yuki24 commented Jun 30, 2017

@schneems I know this is not the best interface, but you could use the :total_pages option and pass in the total number of pages to a #paginate call:

paginate @issues, total_pages: (total_count / per_page).ceil

It will skip calling the #total_pages method which internally calls the #total_count method.

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.

None yet