fix: push limit/offset into the query instead of materializing then slicing #39

Merged
jared merged 1 commits from fix/pushdown-pagination into main 2026-08-06 14:19:42 -04:00
Owner

What broke

cog-api's paginate() sliced an already-fully-materialized result: every page of a deep pagination sweep re-ran the whole cog_spending()/cog_revenue() query and re-listified every row, just to keep up to 1000 and discard the rest. A 193,105-row / 194-page fleet-wide sweep (cog_explorer's Southern guide, corpus-summary build) repeated that full cost 194 times and wedged the production cog-api plumber process for hours on 2026-08-06 — a single request, CPU-bound, single-threaded, and nothing else (not even /health) could get through.

What this does

cog_spending()/cog_revenue() gain optional limit/offset, pushed into .build_verb_sql() as SQL LIMIT/OFFSET behind the existing (already deterministic) ORDER BY. The full unpaginated row count rides along via COUNT(*) OVER() in the same scan, exposed as a total_rows attribute, so a caller walking pages never needs a second round trip to ask how many rows exist. A page now costs O(limit), not O(full result).

Mutually exclusive with complete = TRUE (fills a grid over the FULL requested (year, category) space — pagination over a partial slice of already-grouped rows has no defined meaning for the cells it would fill) and with recipe (a separate, not-yet-wired query path). Both abort with a clear classed condition (uscogdata_complete_pagination_conflict / uscogdata_recipe_pagination_conflict) rather than silently ignoring the parameter.

limit/offset default to NULL; every existing call site is unaffected.

Testing

17 new tests (test-spending-pagination.R, test-revenue-pagination.R): page correctness (no gaps/overlap, walking every page reconstructs the unpaginated result exactly), total_rows accuracy including the offset-past-the-end case, per_capita/adjust_to_year still apply correctly within a page, and both conflict guards.

Full suite: 913/0/0/0 (was 896/0/0/0 baseline).

Companion PR

cog-api's fix/pagination-concurrency-guard wires /spending//revenue up to this — that PR depends on this one merging first (its Docker build clones uscogdata fresh from main).

## What broke cog-api's `paginate()` sliced an already-fully-materialized result: every page of a deep pagination sweep re-ran the whole `cog_spending()`/`cog_revenue()` query and re-listified every row, just to keep up to 1000 and discard the rest. A 193,105-row / 194-page fleet-wide sweep (`cog_explorer`'s Southern guide, corpus-summary build) repeated that full cost 194 times and wedged the production `cog-api` plumber process for hours on 2026-08-06 — a single request, CPU-bound, single-threaded, and nothing else (not even `/health`) could get through. ## What this does `cog_spending()`/`cog_revenue()` gain optional `limit`/`offset`, pushed into `.build_verb_sql()` as SQL `LIMIT`/`OFFSET` behind the existing (already deterministic) `ORDER BY`. The full unpaginated row count rides along via `COUNT(*) OVER()` in the same scan, exposed as a `total_rows` attribute, so a caller walking pages never needs a second round trip to ask how many rows exist. A page now costs `O(limit)`, not `O(full result)`. Mutually exclusive with `complete = TRUE` (fills a grid over the FULL requested `(year, category)` space — pagination over a partial slice of already-grouped rows has no defined meaning for the cells it would fill) and with `recipe` (a separate, not-yet-wired query path). Both abort with a clear classed condition (`uscogdata_complete_pagination_conflict` / `uscogdata_recipe_pagination_conflict`) rather than silently ignoring the parameter. `limit`/`offset` default to `NULL`; every existing call site is unaffected. ## Testing 17 new tests (`test-spending-pagination.R`, `test-revenue-pagination.R`): page correctness (no gaps/overlap, walking every page reconstructs the unpaginated result exactly), `total_rows` accuracy including the offset-past-the-end case, `per_capita`/`adjust_to_year` still apply correctly within a page, and both conflict guards. Full suite: 913/0/0/0 (was 896/0/0/0 baseline). ## Companion PR `cog-api`'s `fix/pagination-concurrency-guard` wires `/spending`/`/revenue` up to this — that PR depends on this one merging first (its Docker build clones `uscogdata` fresh from `main`).
jared added 1 commit 2026-08-06 14:14:28 -04:00
fix: push limit/offset into the query instead of materializing then slicing
R-CMD-check / check (push) Successful in 4m6s
R-CMD-check / check (pull_request) Successful in 3m33s
6a06302036
cog-api's paginate() sliced an already-fully-materialized result: every
page of a deep sweep re-ran the whole cog_spending()/cog_revenue() query
and re-listified every row, just to keep up to 1000 and discard the
rest. A 193,105-row/194-page fleet-wide sweep (cog_explorer's Southern
guide, corpus summary build) repeated that full cost 194 times and
wedged the production server for hours on 2026-08-06 -- single request,
CPU-bound, single-threaded plumber process, no other request could get
through, not even /health.

cog_spending()/cog_revenue() gain optional limit/offset, pushed into
.build_verb_sql() as SQL LIMIT/OFFSET behind the existing (already
deterministic) ORDER BY. The full unpaginated row count rides along via
COUNT(*) OVER() in the same scan -- exposed as a total_rows attribute --
so a caller walking pages never needs a second round trip to ask how
many there are. A page now costs O(limit), not O(full result).

Mutually exclusive with complete = TRUE (which fills a grid over the
FULL requested (year, category) space -- pagination over a partial slice
of already-grouped rows has no defined meaning for the cells it would
fill) and with recipe (whose result comes from a separate, not-yet-wired
query path). Both abort with a clear classed condition rather than
silently ignoring the parameter.

limit/offset default to NULL; every existing call site is unaffected.
jared merged commit c587c8ba87 into main 2026-08-06 14:19:42 -04:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Civilytics/uscogdata#39