cog_spending(), cog_revenue() and cog_balances() gain optional state / type arguments. Both default to NULL, so every existing govid-based call
is byte-identical in behaviour.
Passing them expresses the cohort as a subquery against canonical_fips_xwalk
rather than rendering the ids into a literal IN list. The list was 301,589
characters for type = "city", and .verb_spendrev() embedded it in 5-8
separate statements per call (scope check, main aggregate, per-capita join,
harmonization block, suggestion and suppression queries) -- so it was parsed
and planned from scratch every time it appeared.
Numbers
Production corpus, same FY2022 aggregate over the 20,106-government city
cohort, DUCKDB_THREADS=2, median of 5 (issue #58's profiling figures in
brackets):
cohort expressed as
time
IN (20,106 literals) -- 0.3.0
432 ms [449]
join against a temp cohort table
132 ms [99]
predicate on canonical_fips_xwalk
102 ms [94]
no cohort filter at all (the floor)
105 ms [88]
The predicate lands at the no-filter floor -- within run-to-run noise of
querying with no cohort restriction at all.
End to end through cog_spending(category = "Police"):
time
govid = <20,106 ids>
1080 ms
type = "city"
271 ms
3.99x. Larger than the single-query saving, because the repetition across
statements is what actually cost.
Design decisions
Both made explicitly rather than left as a silent precedence rule:
govid + state/type INTERSECT -- "these ids, narrowed to that
state/type". An error here could never be relaxed later without breaking
callers; the intersection can.
A predicate cohort has no id list to report. provenance$scope$govids_found/govids_missing stay empty and a new scope$cohort block carries state, type and n_governments. Resolving
the ids just to report them would put 20,000 govids into every fleet-scale
response body -- the cost this change removes. A govid-named cohort's
provenance is untouched.
state/type are coerced with .coerce_state_to_fips() / .coerce_type(),
the same helpers cog_gov_search() uses -- the argument is a postal
abbreviation ("WI") while fips_state holds a FIPS code ("55"), and a
predicate on the raw parameter matches nothing and returns an empty result
indistinguishable from "reported nothing".
Drive-by fix
An unknown state now aborts with "Unknown state abbreviation" (class uscogdata_unknown_state) instead of base R's "subscript out of bounds". .state_abbrev_to_fips is a named character vector, so [[ on an absent name
threw before the curated message could be reached -- it was unreachable dead
code in every verb taking a state, cog_gov_search() included. Fixed here
because the new state= argument routes straight into it.
Test plan
testthat::test_local(stop_on_failure = TRUE) (what CI runs) -- 1037 passed, 0 failed, 2 skipped (live-corpus, opt-in). main baseline is 972 passed; the +65 are exactly the new assertions,
so no pre-existing test changed outcome.
Equivalence verified on the production corpus, not just the fixture: govid = <20,106 ids> and type = "city" return 19,236 identical rows.
New test-cohort.R (24) -- SQL construction, postal->FIPS translation,
intersection rendering, alias handling, and a guard that predicate SQL
does not grow with cohort size.
New test-cohort-verbs.R (41) -- equivalence for all three verbs, under per_capita/adjust_to_year/pagination; the intersection rule including
a disjoint intersection returning zero rows; both provenance shapes.
Not in this PR
#59 (/rollups) is not closed by this change, contrary to the note on #58. cog_geographic_rollup() takes govids as a named list of per-layer id
vectors and uses it to build the govid->layer map for its output, so it
structurally needs the ids. Its layers are already exactly state/county/ city -- the same vocabulary as type -- so a predicate-named layer is very
doable, but it changes that verb's public argument shape and deserves its own
issue rather than riding along here.
Adoption in cog-api (behind a formals() probe, as with verb_supports_pagination()) is likewise separate -- nothing needs to deploy
in lockstep.
Closes #58.
## Summary
`cog_spending()`, `cog_revenue()` and `cog_balances()` gain optional `state` /
`type` arguments. Both default to `NULL`, so every existing `govid`-based call
is byte-identical in behaviour.
Passing them expresses the cohort as a subquery against `canonical_fips_xwalk`
rather than rendering the ids into a literal `IN` list. The list was 301,589
characters for `type = "city"`, and `.verb_spendrev()` embedded it in 5-8
separate statements per call (scope check, main aggregate, per-capita join,
harmonization block, suggestion and suppression queries) -- so it was parsed
and planned from scratch every time it appeared.
## Numbers
Production corpus, same FY2022 aggregate over the 20,106-government `city`
cohort, `DUCKDB_THREADS=2`, median of 5 (issue #58's profiling figures in
brackets):
| cohort expressed as | time |
|---|---:|
| `IN (20,106 literals)` -- 0.3.0 | 432 ms [449] |
| join against a temp cohort table | 132 ms [99] |
| predicate on `canonical_fips_xwalk` | **102 ms** [94] |
| no cohort filter at all (the floor) | 105 ms [88] |
The predicate lands **at the no-filter floor** -- within run-to-run noise of
querying with no cohort restriction at all.
End to end through `cog_spending(category = "Police")`:
| | time |
|---|---:|
| `govid = <20,106 ids>` | 1080 ms |
| `type = "city"` | **271 ms** |
**3.99x.** Larger than the single-query saving, because the repetition across
statements is what actually cost.
## Design decisions
Both made explicitly rather than left as a silent precedence rule:
- **`govid` + `state`/`type` INTERSECT** -- "these ids, narrowed to that
state/type". An error here could never be relaxed later without breaking
callers; the intersection can.
- **A predicate cohort has no id list to report.**
`provenance$scope$govids_found`/`govids_missing` stay empty and a new
`scope$cohort` block carries `state`, `type` and `n_governments`. Resolving
the ids just to report them would put 20,000 govids into every fleet-scale
response body -- the cost this change removes. A `govid`-named cohort's
provenance is untouched.
`state`/`type` are coerced with `.coerce_state_to_fips()` / `.coerce_type()`,
the same helpers `cog_gov_search()` uses -- the argument is a postal
abbreviation (`"WI"`) while `fips_state` holds a FIPS code (`"55"`), and a
predicate on the raw parameter matches nothing and returns an empty result
indistinguishable from "reported nothing".
## Drive-by fix
An unknown `state` now aborts with "Unknown state abbreviation" (class
`uscogdata_unknown_state`) instead of base R's "subscript out of bounds".
`.state_abbrev_to_fips` is a named character vector, so `[[` on an absent name
threw before the curated message could be reached -- it was unreachable dead
code in every verb taking a `state`, `cog_gov_search()` included. Fixed here
because the new `state=` argument routes straight into it.
## Test plan
- [x] `testthat::test_local(stop_on_failure = TRUE)` (what CI runs) --
**1037 passed, 0 failed**, 2 skipped (live-corpus, opt-in).
`main` baseline is 972 passed; the +65 are exactly the new assertions,
so no pre-existing test changed outcome.
- [x] `rcmdcheck` `--no-manual --no-vignettes` -- **0 errors, 0 warnings, 0 notes**
- [x] Equivalence verified on the **production** corpus, not just the fixture:
`govid = <20,106 ids>` and `type = "city"` return 19,236 identical rows.
- [x] New `test-cohort.R` (24) -- SQL construction, postal->FIPS translation,
intersection rendering, alias handling, and a guard that predicate SQL
does not grow with cohort size.
- [x] New `test-cohort-verbs.R` (41) -- equivalence for all three verbs, under
`per_capita`/`adjust_to_year`/pagination; the intersection rule including
a disjoint intersection returning zero rows; both provenance shapes.
## Not in this PR
**#59 (`/rollups`) is not closed by this change**, contrary to the note on #58.
`cog_geographic_rollup()` takes `govids` as a named list of per-layer id
vectors and uses it to build the govid->layer map for its output, so it
structurally needs the ids. Its layers are already exactly `state`/`county`/
`city` -- the same vocabulary as `type` -- so a predicate-named layer is very
doable, but it changes that verb's public argument shape and deserves its own
issue rather than riding along here.
Adoption in cog-api (behind a `formals()` probe, as with
`verb_supports_pagination()`) is likewise separate -- nothing needs to deploy
in lockstep.
cog_spending(), cog_revenue() and cog_balances() gain optional state/type
arguments. Both default to NULL, so every existing govid-based call is
unchanged.
The verbs took a cohort only as a govid vector, which .sql_lit_chr()
rendered into a quoted IN list and .verb_spendrev() embedded into 5-8
separate statements per call: the scope check, the main aggregate, the
per-capita join, the harmonization block, and the suggestion and
suppression queries. For type = "city" that list is 301,589 characters,
parsed and planned from scratch every time it appears.
Passing state/type instead expresses the cohort as a subquery against
canonical_fips_xwalk, so its size never enters the SQL string at all.
Measured on the production corpus, same FY2022 aggregate over the
20,106-government city cohort, DUCKDB_THREADS=2, median of 5:
IN (20,106 literals) -- 0.3.0 432 ms
join against a temp cohort table 132 ms
predicate on canonical_fips_xwalk 102 ms
no cohort filter at all (the floor) 105 ms
The predicate reaches the no-filter floor: the cohort restriction is
now free. End to end through cog_spending(category = "Police"),
1080 ms -> 271 ms, 3.99x -- larger than the single-query saving,
because the repetition across statements is what actually cost.
Design decisions, both made explicitly rather than left implicit:
- govid AND state/type INTERSECT. "These ids, narrowed to that
state/type" is a real query, and an error here could never be
relaxed later without breaking callers.
- A predicate cohort has no id list to report, so
provenance$scope$govids_found/govids_missing stay empty and a new
scope$cohort block carries state, type and n_governments. Resolving
the ids just to report them would put 20,000 govids in every
fleet-scale response body -- the cost this change removes. A
govid-named cohort's provenance is untouched.
state/type are coerced with .coerce_state_to_fips()/.coerce_type(), the
same helpers cog_gov_search() uses. That is load-bearing: the argument
is a postal abbreviation ("WI") while fips_state holds a FIPS code
("55"), and a predicate on the raw parameter matches nothing and returns
an empty result indistinguishable from "reported nothing". cog-api hit
exactly this trap optimizing the same path.
.attach_per_capita() now keys its population lookup on the govids present
in the result rather than the requested cohort. Those are the only ones
its LEFT JOIN can match, so the output is identical -- but it needs no id
list, and on a paginated call it looks up one page instead of the fleet.
Fixes uscogdata#58.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #58.
Summary
cog_spending(),cog_revenue()andcog_balances()gain optionalstate/typearguments. Both default toNULL, so every existinggovid-based callis byte-identical in behaviour.
Passing them expresses the cohort as a subquery against
canonical_fips_xwalkrather than rendering the ids into a literal
INlist. The list was 301,589characters for
type = "city", and.verb_spendrev()embedded it in 5-8separate statements per call (scope check, main aggregate, per-capita join,
harmonization block, suggestion and suppression queries) -- so it was parsed
and planned from scratch every time it appeared.
Numbers
Production corpus, same FY2022 aggregate over the 20,106-government
citycohort,
DUCKDB_THREADS=2, median of 5 (issue #58's profiling figures inbrackets):
IN (20,106 literals)-- 0.3.0canonical_fips_xwalkThe predicate lands at the no-filter floor -- within run-to-run noise of
querying with no cohort restriction at all.
End to end through
cog_spending(category = "Police"):govid = <20,106 ids>type = "city"3.99x. Larger than the single-query saving, because the repetition across
statements is what actually cost.
Design decisions
Both made explicitly rather than left as a silent precedence rule:
govid+state/typeINTERSECT -- "these ids, narrowed to thatstate/type". An error here could never be relaxed later without breaking
callers; the intersection can.
provenance$scope$govids_found/govids_missingstay empty and a newscope$cohortblock carriesstate,typeandn_governments. Resolvingthe ids just to report them would put 20,000 govids into every fleet-scale
response body -- the cost this change removes. A
govid-named cohort'sprovenance is untouched.
state/typeare coerced with.coerce_state_to_fips()/.coerce_type(),the same helpers
cog_gov_search()uses -- the argument is a postalabbreviation (
"WI") whilefips_stateholds a FIPS code ("55"), and apredicate on the raw parameter matches nothing and returns an empty result
indistinguishable from "reported nothing".
Drive-by fix
An unknown
statenow aborts with "Unknown state abbreviation" (classuscogdata_unknown_state) instead of base R's "subscript out of bounds"..state_abbrev_to_fipsis a named character vector, so[[on an absent namethrew before the curated message could be reached -- it was unreachable dead
code in every verb taking a
state,cog_gov_search()included. Fixed herebecause the new
state=argument routes straight into it.Test plan
testthat::test_local(stop_on_failure = TRUE)(what CI runs) --1037 passed, 0 failed, 2 skipped (live-corpus, opt-in).
mainbaseline is 972 passed; the +65 are exactly the new assertions,so no pre-existing test changed outcome.
rcmdcheck--no-manual --no-vignettes-- 0 errors, 0 warnings, 0 notesgovid = <20,106 ids>andtype = "city"return 19,236 identical rows.test-cohort.R(24) -- SQL construction, postal->FIPS translation,intersection rendering, alias handling, and a guard that predicate SQL
does not grow with cohort size.
test-cohort-verbs.R(41) -- equivalence for all three verbs, underper_capita/adjust_to_year/pagination; the intersection rule includinga disjoint intersection returning zero rows; both provenance shapes.
Not in this PR
#59 (
/rollups) is not closed by this change, contrary to the note on #58.cog_geographic_rollup()takesgovidsas a named list of per-layer idvectors and uses it to build the govid->layer map for its output, so it
structurally needs the ids. Its layers are already exactly
state/county/city-- the same vocabulary astype-- so a predicate-named layer is verydoable, but it changes that verb's public argument shape and deserves its own
issue rather than riding along here.
Adoption in cog-api (behind a
formals()probe, as withverb_supports_pagination()) is likewise separate -- nothing needs to deployin lockstep.
cog_spending(), cog_revenue() and cog_balances() gain optional state/type arguments. Both default to NULL, so every existing govid-based call is unchanged. The verbs took a cohort only as a govid vector, which .sql_lit_chr() rendered into a quoted IN list and .verb_spendrev() embedded into 5-8 separate statements per call: the scope check, the main aggregate, the per-capita join, the harmonization block, and the suggestion and suppression queries. For type = "city" that list is 301,589 characters, parsed and planned from scratch every time it appears. Passing state/type instead expresses the cohort as a subquery against canonical_fips_xwalk, so its size never enters the SQL string at all. Measured on the production corpus, same FY2022 aggregate over the 20,106-government city cohort, DUCKDB_THREADS=2, median of 5: IN (20,106 literals) -- 0.3.0 432 ms join against a temp cohort table 132 ms predicate on canonical_fips_xwalk 102 ms no cohort filter at all (the floor) 105 ms The predicate reaches the no-filter floor: the cohort restriction is now free. End to end through cog_spending(category = "Police"), 1080 ms -> 271 ms, 3.99x -- larger than the single-query saving, because the repetition across statements is what actually cost. Design decisions, both made explicitly rather than left implicit: - govid AND state/type INTERSECT. "These ids, narrowed to that state/type" is a real query, and an error here could never be relaxed later without breaking callers. - A predicate cohort has no id list to report, so provenance$scope$govids_found/govids_missing stay empty and a new scope$cohort block carries state, type and n_governments. Resolving the ids just to report them would put 20,000 govids in every fleet-scale response body -- the cost this change removes. A govid-named cohort's provenance is untouched. state/type are coerced with .coerce_state_to_fips()/.coerce_type(), the same helpers cog_gov_search() uses. That is load-bearing: the argument is a postal abbreviation ("WI") while fips_state holds a FIPS code ("55"), and a predicate on the raw parameter matches nothing and returns an empty result indistinguishable from "reported nothing". cog-api hit exactly this trap optimizing the same path. .attach_per_capita() now keys its population lookup on the govids present in the result rather than the requested cohort. Those are the only ones its LEFT JOIN can match, so the output is identical -- but it needs no id list, and on a paginated call it looks up one page instead of the fleet. Fixes uscogdata#58.