Skip to content

clamp the last page end index to the row count - #197

Open
arpitjain099 wants to merge 1 commit into
CDCgov:mainfrom
arpitjain099:fix/get-pages-clamp-end-index
Open

arpitjain099 wants to merge 1 commit into
CDCgov:mainfrom
arpitjain099:fix/get-pages-clamp-end-index

Conversation

@arpitjain099

Copy link
Copy Markdown

Setting limit on a Query makes get_pages() raise AssertionError: End index 9999 is larger than limit 5. The loop computes the page end as (i + 1) * page_size - 1 and never clamps it to the row count it just resolved, so the last page asks for rows that are not there. With the default page_size of 10,000, any limit below 10,000 fails on the first page, and any limit that is not an exact multiple of the page size fails on the last one. Taking min() against row_count fixes both, and it also stops the final page asking for a whole page of rows when no limit is set.

One existing assertion changed with it. test_get_pages_yields_expected_page_ranges expected end=3 on a three-row dataset, which is a row index that does not exist. I added three tests that drive get_pages through the real _get_records.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>

This branch has not been deployed

No deployments
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.

1 participant