-
-
Notifications
You must be signed in to change notification settings - Fork 899
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
Comments
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. |
Something I didn't find mentioned directly in the other pagination issue reports is that from flask_sqlalchemy import SQLAlchemy, BaseQuery
db = SQLAlchemy(session_options={'query_cls': BaseQuery}) |
Making a note here that |
I am sure I am using flask_sqlalchemy, but "AttributeError: 'list' object has no attribute 'paginate'" happens. so upset! |
@wck821829906: Don't call |
Has there been any progress on this issue? I am not at all an SQL expert, otherwise I would try to tackle it ... |
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. |
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. |
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. |
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 |
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. |
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). |
#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 |
In #1100 I ended up removing creating |
Devise a more extensible solution which deals with the issues raised in #232, #281, #265, #151 & #70
The text was updated successfully, but these errors were encountered: