REST converters: page by id windows, and never cache what is not a page - #232
Open
ivorbosloper wants to merge 2 commits into
Open
REST converters: page by id windows, and never cache what is not a page#232ivorbosloper wants to merge 2 commits into
ivorbosloper wants to merge 2 commits into
Conversation
Paging by `sortBy` + `attribute > last_id` asks the server to sort the whole layer for every page. On Cantabria's joined SIGPAC layers that costs about 100 seconds per request — days for one edition — where a range filter on the same indexed key answers in one or two. Galicia is 15.8 million recintos per edition, in pages of 1000. Paging by half-open id windows removes the sort: the key is unique, so a window of `page_size` ids cannot overflow a page, and gaps only produce empty windows, which are skipped. Pages cached by the old scheme are keyed by the previous page's last id; on a dense layer those coincide exactly with a window, so one is reused when its own ids prove it covers the window completely. Three more things this turned up, each invisible in the output: - **Layer ids repeat across services.** Every SIXPAC_<year> MapServer has its Recintos layer at id 2, so a cache keyed on the layer alone served one year's pages for another. The service is part of the key now. - **An error response was cached as if it were data.** Esri answers a failed query with 200 and a JSON error body; that body was written to the cache file and every later run read it back. So was a download that broke off halfway. Neither survives now. - **Joined layers qualify every field with its table name**, so "OBJECTID" matches nothing and the paging filter silently returns everything. The real key field is discovered from a one-row probe before paging starts. `rest_format`, the fixture-reading branch and the way `rest_params["where"]` is combined with the paging filter are unchanged; the test that pins them is updated for the new clause, which now reads `OBJECTID>0 AND OBJECTID<=1000 AND (<the converter's own filter>)`. tests/test_converter_rest.py serves a small fake service of five features in pages of two, which is enough to hold every one of these: the window arithmetic, the cache naming, the two guards, the qualified key, and the reuse of a legacy page only when its ids cover the window. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG
m-mohr
reviewed
Sep 11, 2026
Co-authored-by: Matthias Mohr <m.mohr@moregeo.it>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of the example PRs splitting
publish-portolan(the first is #231, Slovenia). This one is structural: the Esri REST paging mixin and its tests.Note this is a merge, not a copy.
mainhas moved on since the branch forked —rest_format, the deliberate fixture-reading branch, the combination ofrest_params["where"]with the paging filter, andtest_rest_query_paramsthat pins them. All of that is kept; only the paging changes, and that test is updated for the new clause.Why
Paging by
sortBy+attribute > last_idasks the server to sort the whole layer for every page. On Cantabria's joined SIGPAC layers that costs ~100 s per request — days for one edition — where a range filter on the same indexed key answers in one or two. Galicia is 15.8M recintos per edition, in pages of 1000.Half-open id windows remove the sort: the key is unique, so a window of
page_sizeids cannot overflow a page, and gaps only produce empty windows. Pages cached by the old scheme are keyed by the previous page's last id, which on a dense layer coincides exactly with a window, so one is reused when its own ids prove it covers the window completely — no re-download of what is already there.Three bugs this turned up, none visible in the output
SIXPAC_<year>MapServer has its Recintos layer at id 2, so a cache keyed on the layer alone served one year's pages for another. The service is part of the key nowRECINTOS.OBJECTID, soOBJECTIDmatches nothing and the paging filter silently returns everything. The real key is discovered from a one-row probe before paging startsTests
tests/test_converter_rest.pyserves a fake service of five features in pages of two — enough to hold the window arithmetic, the cache naming, both guards, the qualified key, and the legacy-page reuse (including the case where the ids do not cover the window, so it must re-fetch). Coverage of the mixin goes from 19% to 94%.Original commits on
publish-portolan: 48fe60e, 6b5a373, bd45156, 32171c7, 0637b5a. In use for the Cantabria (16 editions) and Galicia (13 editions) backfills in the harmonized field data catalog.🤖 Generated with Claude Code
https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG