Skip to content

More efficient way to detect next page#15

Merged
dannysepler merged 5 commits into
mainfrom
danny/more-efficient-next-page
Feb 4, 2026
Merged

More efficient way to detect next page#15
dannysepler merged 5 commits into
mainfrom
danny/more-efficient-next-page

Conversation

@dannysepler

@dannysepler dannysepler commented Feb 4, 2026

Copy link
Copy Markdown

Context

We're running a secret inefficient count when loading the Providers User admin page. This should hopefully do this query more efficiently

Comment thread flask_peewee/rest/filters.py Outdated

def has_next_page(self):
# Running count() as done in get_pages() can be inefficient for un-indexed queries.
with_one_more = self.query.paginate(self.get_page(), self.paginate_by + 1)

@achien achien Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong because it will offset by (page number) * (page size) and this makes the page size bigger

I think what we can do is just fetch the next page with self.query.paginate(self.get_page() + 1, self.paginate_by)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes of course! that makes sense

@achien achien left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, with two nits

def has_results_on_next_page(self):
# Running count() as done in get_pages() can be inefficient for un-indexed queries.
results = self.query.paginate(self.get_page() + 1, self.paginate_by)
return len(results) > 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nits: I think paginate returns the query object, so results is actually a query and means we can also use exists() here if we want

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this failed tests, so i may just keep it the old way. sorry, just trying to not get tooooo distracted by this

Comment thread setup.py Outdated
setup(
name='flask-peewee',
version='3.0.5+propel',
version='3.0.6+propel',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this propel.2 instead of bumping the version, so we can merge the upstream 3.0.6 in the future if it exists?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh true. anyway it does exist! he bumped to 3.1.0 like last month

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll just stay on 3.0.5+propel, our UV pinning goes by commit hash anyway

@dannysepler dannysepler merged commit 976face into main Feb 4, 2026
4 checks passed
@dannysepler dannysepler deleted the danny/more-efficient-next-page branch February 4, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants