Navigation Menu

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

Revisit Pagination #282

Closed
immunda opened this issue Apr 24, 2015 · 14 comments
Closed

Revisit Pagination #282

immunda opened this issue Apr 24, 2015 · 14 comments
Milestone

Comments

@immunda
Copy link
Contributor

immunda commented Apr 24, 2015

Devise a more extensible solution which deals with the issues raised in #232, #281, #265, #151 & #70

@immunda immunda self-assigned this Apr 24, 2015
@immunda immunda added this to the v3.0 milestone Apr 24, 2015
@xen
Copy link

xen commented Apr 27, 2015

I put some thoughts into #272, and think it is worth to mention here. OFFSET is not the best way to paginate. It was also noticed by @noah in #232 (comment). So, I like to join discussion.

@davidism
Copy link
Member

Something I didn't find mentioned directly in the other pagination issue reports is that db.session does not use the same query class as db.Model.query. This is trivial to solve by passing query_cls, but it's a weird oversight that can cause confusion. For example, db.session.query(thing1, thing2).paginate() doesn't work.

from flask_sqlalchemy import SQLAlchemy, BaseQuery
db = SQLAlchemy(session_options={'query_cls': BaseQuery})

@immunda
Copy link
Contributor Author

immunda commented Dec 4, 2015

Making a note here that db.session now uses the same query class as db.Model.query by default as of #328 💃

@wck821829906
Copy link

I am sure I am using flask_sqlalchemy, but "AttributeError: 'list' object has no attribute 'paginate'" happens. so upset!

@ThiefMaster
Copy link
Contributor

@wck821829906: Don't call .all() before calling .paginate(). Also, don't hijack unrelated issues.

@sebpiq
Copy link

sebpiq commented Jun 8, 2017

Has there been any progress on this issue? I am not at all an SQL expert, otherwise I would try to tackle it ...

@patrickyan
Copy link

COUNT is super slow, and Flask-SQLAlchemy always gets COUNT for pagination. The first step would be to stop using COUNT and just have prev/next based pagination. Since this project looks dead, we just have to create our own implementations.

@davidism
Copy link
Member

davidism commented Sep 3, 2017

Hi Patrick, this project is not dead. I'd be happy to review any work you're willing to put into pull requests, but have limited free time to devote to this myself at the moment. Thank you.

@davidism
Copy link
Member

davidism commented Sep 3, 2017

As to the count stuff, this comes up every now and then, search back through the closed issues for an explanation. I don't plan to remove it.

@ThiefMaster
Copy link
Contributor

There are many cases where COUNT and OFFSET are perfectly fine. Unless you are above ten thousands of rows in the initial resultset you can safely use them. And honestly, most typical webapps using flask-sqlalchemy's pagination are not that likely to end up having to paginate that many rows.

If you need something more efficient (usually prev/next based on WHERE indexed-column > prev-value LIMIT x and similar approaches writing your own implementation is easy.

@Birne94
Copy link

Birne94 commented Nov 23, 2018

Maybe using window functions might improve performance here? I noticed that (at least for us and using postgres), counting through window functions results in faster queries (tested for tables with around 10 million rows and especially queries containing joins).

A query like

SELECT foo.*, count(*) OVER () AS total
FROM mytable
ORDER BY somecolumn DESC;

performs way better than two distinct queries

SELECT foo.*
FROM mytable
ORDER BY somecolumn DESC
LIMIT 20
OFFSET 0;

SELECT count(*)
FROM mytable

I am, however, not sure how to implement this in a generic way to include the total count when querying the relevant items.

@rsyring
Copy link
Contributor

rsyring commented Mar 8, 2019

This may be way out in left field but...should we consider removing pagination from Flask-SQLAlchemy? My brief thought is that FSA is a bridge between Flask and SQLAlchemy. Pagination seems like a separate concern. Is there anything that requires Pagination to be embedded in the library as opposed to maintained in a separate library (assuming someone wanted to do that)?

I bring this up mostly due to the maintenance burden of having additional code in the library. As I went through the issues today, there were a decent number of them that touched on pagination. I'd personally rather see core FSA get more love and bump this out to a separate library (or just deprecate and then eventually remove it and let someone else build the library if they want to maintain it separate from FSA).

@davidism
Copy link
Member

#1087 addressed some pagination issues, I'm fine with continuing to support it.

The query interface is legacy in SQLAlchemy 2.0, I'll be adding some support for select() instead.

@davidism
Copy link
Member

In #1100 I ended up removing creating Pagination objects manually as a public API. I think supporting pagination makes sense, it's a very common interface in web applications, but I also don't think we need to provide a generic pagination API. So now db.paginate and query.paginate are the only ways to get pagination in the public API.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

9 participants