pallets-eco / flask-sqlalchemy

Adds SQLAlchemy support to Flask
https://flask-sqlalchemy.readthedocs.io
BSD 3-Clause "New" or "Revised" License
4.24k stars 901 forks source link

Support select(Model.column1, Model.column2) in pagination #1168

Open drsanwujiang opened 1 year ago

drsanwujiang commented 1 year ago

1100 says that db.paginate currently not work for select(Model.column1, Model.column2), so will this feature going to be implemented in a future version, or what could I do to get this?

jfnhs57 commented 1 year ago

I am having the same issues upgrading to SQLAlchemy 2.0.x with lots of Model.query.with_entities(...).filter().order_by().paginate().

All these seem to return only the id per row, not the remaining fields in with_entities(...)

In the code of pagination.py I found this on line 335:

when I change it to:

And use

and

then I do get the columns with dict(d._mapping) for d in res.

Not sure, though, if this is the correct solution for all situation?

demenr commented 1 year ago

I would also like to have this fixed, it seems to me a regression. It was working fine with SQLAlchemy 1.4 version and Flask-SQLAlchemy 2.x

davidism commented 1 year ago

Because SQLAlchemy doesn't provide a way to introspect whether a query will return a model instance or rows. The primary use case is select(User) or User.query, where users expect to receive a list of unique instances back, not a list of 1-tuples. Happy to consider a PR that allows supporting both, but you'll probably need to reach out to SQLAlchemy first to get a public API to enable that.

From the docs:

The statement should select a model class, like select(User). This applies unique() and scalars() modifiers to the result, so compound selects will not return the expected results.

instanceofmel commented 1 year ago

I'm using the legacy query object too for now, because SelectPagination doesn't support these compound selects. Would love to see this implemented!

davidism commented 1 year ago

Happy to review a PR. Just saying "me too" isn't helpful though. This is an community open source project. If a feature is important to you, you can implement it.

jwodder commented 1 year ago

@davidism

If a feature is important to you, you can implement it.

OK, done: https://github.com/pallets-eco/flask-sqlalchemy/pull/1269

davidism commented 1 year ago

Thanks for working on it. That's an easy to implement new method. But that doesn't solve the problem that people will call paginate first and then still think there's an issue. Based on what people have asked for, it sounds like they want one method that handles both cases automatically. That's what we need help with, someone to investigate how to do that, or follow up with SQLAlchemy about making it possible.

jwodder commented 1 year ago

@davidism At the moment, I believe the primary issue is that there isn't a way to paginate compound selects without either using the legacy Query or rolling your own pagination. Adding paginate_rows() solves this, and after that, there's really no need for paginate() to do case autodetection, as the programmer can just call the appropriate method themselves. Yes, some people will miss the note in the documentation telling them to use paginate_rows() if they have a compound select, but that's true of all documentation.

jfnhs57 commented 11 months ago

I am just wondering if there is a way to preserve the column names on the paginate_rows() method? I know I can use the _mapping, but this is not the proper way?

jpstotz commented 2 weeks ago

@davidism I agree with jwodder. At the moment flask-sqlalchemy is defect as there is no way to paginate queries that select individual columns.

Without it many users are stuck at flask-sqlalchemy 2.5.1 as rewriting complex sqlalchemy queries is not an option just to be able to do pagination again. So please accept paginate_rows() or provide a better solution.