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

Wrong number of items in pagination if not unique (and should unique be enforced?) #1382

Open
joao-aveiro opened this issue Jan 17, 2025 · 1 comment

Comments

@joao-aveiro
Copy link

joao-aveiro commented Jan 17, 2025

Description

By default, the pagination query methods enforce .unique() on the session/execute level. Since limit/offset is done beforehand at the query level, if the retrieved elements are not unique, the selected objects will not reflect the uniqueness/distinct filter and the query will return less elements than defined by per_page. See the code below.

class SelectPagination(Pagination):
    """Returned by :meth:`.SQLAlchemy.paginate`. Takes ``select`` and ``session``
    arguments in addition to the :class:`Pagination` arguments.

    .. versionadded:: 3.0
    """

    def _query_items(self) -> list[t.Any]:
        select = self._query_args["select"]
        select = select.limit(self.per_page).offset(self._query_offset)  # **The pagination limits are applied to the select statement**
        session = self._query_args["session"]
        return list(session.execute(select).unique().scalars())  # **But only here the `.unique()` is applied**

(...) 

On a side-note, I don't think .unique() should be enforced at this level; I believe this should be achieved manually by the user at the select statement level or, at least, be optional based on an attribute in the Pagination class. For me, at least, the current behaviour is highly undesireable because 1) I might want non-unique objects, and 2) it might obfuscate problems I might have in my query.

Please consider adding this flag for now for users to opt-out of this (to avoid breaking deployments) and possibly remove this filter altogether in a future major (breaking) change.

Fix

  • Apply .distinct() in the select statement and remove . unique() from the session.execute
  • (Optional) Make this unique filter optional

Environment

  • Python version: 3.11
  • Flask-SQLAlchemy version: main branch (3e3e92b), latest release 3.1.1
  • SQLAlchemy version: 2.0.36
@joao-aveiro
Copy link
Author

I can make a quick PR for this if the maintainers are willing to address this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant