diff --git a/R/spending.R b/R/spending.R index eccb958..cfefe90 100644 --- a/R/spending.R +++ b/R/spending.R @@ -49,7 +49,12 @@ #' to the state government (`L` codes, excluding the `L--` family-total #' rollup) -- so results gain rows with `spend_subtype == #' "intergovernmental"`. Mutually exclusive with `recipe` (a recipe -#' already defines its own component codes). +#' already defines its own component codes). **Do not sum `"total"` +#' results across levels of government** (e.g. state + county + city): +#' a state's `M12` payment to a school district is the same dollar the +#' district reports as its own direct `E12`, so summing both double-counts +#' it. This matters in particular with [cog_geographic_rollup()], which +#' sums across exactly that kind of multi-layer government set. #' @return Tibble with columns `year`, `canonical_govid`, `gov_name`, #' `spend_subtype`, `category`, `amt_nominal`, optional `amt_real`, #' optional `amt_per_capita_nominal`, optional `amt_per_capita_real`, @@ -111,6 +116,24 @@ cog_spending <- function(govid, years, category = NULL, ), class = "uscogdata_recipe_concept_conflict") } + # .verb_spendrev() is shared with cog_revenue(), which never exposes + # expenditure_concept and always resolves it to "direct" -- so nothing on + # the public API can reach this today. But it's a cheap guard against a + # future call (direct or via a modified cog_revenue()) that would UNION + # the IG leg's expenditure M/L rows into a revenue result, which has no + # matching IG view and no sensible meaning. + if (identical(expenditure_concept, "total") && + !identical(view_base, "spending_annotated")) { + cli::cli_abort( + paste0( + "`expenditure_concept = \"total\"` is only supported for spending ", + "(view_base = \"spending_annotated\"); got view_base = ", + "{.val {view_base}}." + ), + class = "uscogdata_expenditure_concept_unsupported" + ) + } + years <- as.integer(years) if (!is.null(adjust_to_year)) adjust_to_year <- as.integer(adjust_to_year) @@ -280,6 +303,16 @@ cog_spending <- function(govid, years, category = NULL, sprintf("(SELECT * FROM %s UNION ALL SELECT * FROM %s)", view, ig_view) } + # bool_or(), not bool_and(): a no-op for the Direct/revenue legs (those + # views filter NOT is_aggregate, so no row in any group is ever aggregate), + # but load-bearing for the IG leg, which deliberately keeps aggregate rows + # (see inst/sql/24-ig_long.sql). The wide era is dense -- every government + # has a row for every code in a family, most of them $0 -- so a $0 leaf + # commonly lands in the same (year, gov, subtype, category) group as the + # real aggregate row. bool_and() would then read FALSE for that group even + # though its dollars came entirely from an aggregate row, silently + # suppressing the "Aggregate fallback applied" note on exactly the rows + # this feature exists to surface. sprintf( "SELECT year, @@ -289,7 +322,7 @@ cog_spending <- function(govid, years, category = NULL, category, SUM(amt) * 1000.0 AS amt_nominal, string_agg(DISTINCT item_code, ',' ORDER BY item_code) AS codes_included, - bool_and(is_aggregate) AS aggregate_fallback + bool_or(is_aggregate) AS aggregate_fallback FROM %2$s WHERE canonical_govid IN (%3$s) AND year IN (%4$s) diff --git a/R/views.R b/R/views.R index 6a4bdf4..c5febdd 100644 --- a/R/views.R +++ b/R/views.R @@ -1,15 +1,25 @@ # R/views.R -# SQL files whose view definitions read schema-v5-only parquet tables -# (harmonization_map.parquet, harmonization_recipes.parquet, -# series_breaks.parquet) or select from views built on top of them. DuckDB's -# read_parquet() resolves the file at CREATE VIEW time (even for a view, it -# still needs the source schema) and errors immediately -- "IO Error: No -# files found" -- if the path doesn't exist, so these cannot be registered -# unconditionally against a v4 corpus the way the rest of inst/sql/ is. -# Registration is therefore gated on manifest$schema_version >= 5; verb-level -# *usage* of the resulting views is separately gated by .resolve_basis() / -# .require_schema_v5(). +# SQL files that cannot be registered unconditionally against a v4 corpus, +# for one of two distinct reasons -- both fail at CREATE VIEW time (DuckDB +# resolves a view's source schema eagerly, even though it defers execution), +# so a v4 corpus can't tolerate either unconditionally: +# +# (a) Missing FILE. 33-/34-/35- read_parquet() a v5-only parquet table +# (harmonization_map.parquet, harmonization_recipes.parquet, +# series_breaks.parquet) that doesn't exist at all on a v4 corpus -- +# "IO Error: No files found". +# +# (b) Missing COLUMN. 22-/23-/25- reference `long.harmonized_code`, a +# column that does not exist on a v4 corpus's `long` table (harmonized +# space was introduced in schema v5) -- "Binder Error: Referenced +# column harmonized_code not found". 42-/43-/45- are on this list only +# because they SELECT s.* FROM the (a)/(b) views above, so they'd fail +# to resolve their own source view if it weren't already skipped. +# +# Registration is therefore gated on manifest$schema_version >= 5 for all of +# them; verb-level *usage* of the resulting views is separately gated by +# .resolve_basis() / .require_schema_v5(). .harmonization_view_files <- c( "22-spending_long_harmonized.sql", "23-revenue_long_harmonized.sql", diff --git a/inst/sql/25-ig_long_harmonized.sql b/inst/sql/25-ig_long_harmonized.sql index 08504b2..5187265 100644 --- a/inst/sql/25-ig_long_harmonized.sql +++ b/inst/sql/25-ig_long_harmonized.sql @@ -2,9 +2,12 @@ -- than harmonized_code alone: aggregate rows carry NO harmonized_code by -- construction (harmonized space is leaf-only), so a plain -- `harmonized_code IS NOT NULL` filter would drop every legacy IG aggregate -- --- 6.4e9 of M and 3.1e8 of L in corpus units. COALESCE keeps the one real IG --- collapse rule (M38 -> M36, SB012, year-disjoint 1967-2011 vs 2012+) while --- never dropping a row. +-- in the bundled fixture corpus (year 2011; 2012+ all carry a harmonized_code) +-- that is $379,016,063k across 25,688 M rows and $2,277,458k across 19,266 L +-- rows (`SELECT year, LEFT(item_code,1), SUM(amt), COUNT(*) FROM ig_long +-- WHERE harmonized_code IS NULL GROUP BY 1, 2`). COALESCE keeps the one real +-- IG collapse rule (M38 -> M36, SB012, year-disjoint 1967-2011 vs 2012+) +-- while never dropping a row. CREATE OR REPLACE VIEW ig_long_harmonized AS SELECT * REPLACE (COALESCE(harmonized_code, item_code) AS item_code) FROM long diff --git a/man/cog_spending.Rd b/man/cog_spending.Rd index ce9edbe..d129e97 100644 --- a/man/cog_spending.Rd +++ b/man/cog_spending.Rd @@ -64,7 +64,12 @@ intergovernmental leg -- payments to local governments (`M` codes) and to the state government (`L` codes, excluding the `L--` family-total rollup) -- so results gain rows with `spend_subtype == "intergovernmental"`. Mutually exclusive with `recipe` (a recipe -already defines its own component codes).} +already defines its own component codes). **Do not sum `"total"` +results across levels of government** (e.g. state + county + city): +a state's `M12` payment to a school district is the same dollar the +district reports as its own direct `E12`, so summing both double-counts +it. This matters in particular with [cog_geographic_rollup()], which +sums across exactly that kind of multi-layer government set.} } \value{ Tibble with columns `year`, `canonical_govid`, `gov_name`, diff --git a/tests/testthat/test-expenditure-concept.R b/tests/testthat/test-expenditure-concept.R index cf5f86f..5609ba8 100644 --- a/tests/testthat/test-expenditure-concept.R +++ b/tests/testthat/test-expenditure-concept.R @@ -29,8 +29,12 @@ test_that("expenditure_concept = 'total' adds an intergovernmental subtype", { t <- cog_spending(gov, years = 2019, category = "Police", expenditure_concept = "total") expect_true("intergovernmental" %in% t$spend_subtype) - # Direct rows are untouched; Total only ever ADDS. - dt <- t[t$spend_subtype != "intergovernmental", ] + # Direct rows are untouched; Total only ever ADDS. Use %in% rather than + # != : a category = NULL result can contain a NULL-subtype group (codes + # with no summary_categories row, e.g. E16/E21/E85/F16/F85/G16/G21/G85), + # and `NA != "intergovernmental"` is NA, not TRUE, which would silently + # smuggle an all-NA phantom row into dt. + dt <- t[!(t$spend_subtype %in% "intergovernmental"), ] expect_equal(sort(dt$amt_nominal), sort(d$amt_nominal)) expect_gt(sum(t$amt_nominal), sum(d$amt_nominal)) }) @@ -85,3 +89,100 @@ test_that("recipe = and expenditure_concept = 'total' together aborts", { class = "uscogdata_recipe_concept_conflict" ) }) + +test_that("aggregate-sourced IG dollars are flagged aggregate_fallback = TRUE (bool_or, not bool_and)", { + # Regression test: .build_verb_sql() originally used bool_and(is_aggregate) + # for aggregate_fallback, which is correct for the Direct leg (a group can + # never mix aggregate and non-aggregate rows there -- spending_long filters + # NOT is_aggregate) but wrong for the IG leg. The wide era is dense -- every + # government has a $0 row for every code in a family -- so a $0 leaf sits in + # the same (year, gov, subtype, category) group as the real aggregate row + # and flips bool_and() to FALSE. Measured: AL state 2011 had $5,740,775,000 + # of aggregate-sourced IG dollars (Corrections $31,358,000 + Education K-12 + # $5,152,385,000 + General Government $557,032,000) reporting + # aggregate_fallback = FALSE under bool_and(), with the only TRUE row being + # Transit Utilities at $0. bool_or() reports all of them correctly. + gov <- "010000226085" + t <- cog_spending(gov, years = 2011, category = "Education K-12", + expenditure_concept = "total") + ig <- t[t$spend_subtype == "intergovernmental", ] + expect_equal(nrow(ig), 1L) + expect_true(ig$aggregate_fallback) + expect_true(nzchar(ig$notes)) + expect_match(ig$notes, "Aggregate fallback applied", fixed = TRUE) +}) + +test_that("legacy aggregate IG codes are year-disjoint from their modern leaf components", { + # The safety of ig_long's deliberate omission of `NOT is_aggregate` (see + # inst/sql/24-ig_long.sql) rests entirely on each legacy code's AGGREGATE + # instance being year-disjoint from the modern leaf codes it rolls up -- + # if a future corpus rebuild ever back-filled a leaf into a year where the + # code is still flagged aggregate, `total` would silently double-count and + # this suite would still pass. This test fails loudly if that ever + # happens. + # + # Note the invariant is scoped to the AGGREGATE flag, not bare code + # presence: M89/L89 do NOT disappear after the wide era the way M47/L47 + # do -- they continue past 2011 as their OWN independent leaf line item + # (is_aggregate = FALSE) alongside M91-93/L91-93, which is fine because a + # non-aggregate M89/L89 no longer represents a rollup of those codes. + # (Verified in the fixture: M89/L89 are is_aggregate = TRUE only in 2011, + # when M91-93/L91-93 don't exist yet; from 2012 on M89/L89 are + # is_aggregate = FALSE leaves coexisting with M91-93/L91-93.) + # + # Pairs are the M/L-prefixed components (this package's ig_long only + # covers M/L; other prefixes in the same rollup, e.g. N/O/P/Q/R, fall + # outside its domain and are irrelevant here) enumerated in + # cog_pipeline's data/wide_to_long_xwalk.csv `full_desc` column (read + # once at authoring time, not at test time -- this test stays offline): + # M47 "To local governments, total (includes N47, O47, P47, R47, and M94)" + # M89 "To local governments, total (incl N89, O89, P89, R89, M91, M92, and M93)" + # L47 "To state government (includes L94)" + # L89 "To state government (includes L91, L92, and L93)" + con <- .ensure_session() + pairs <- list( + list(aggregate = "M47", components = "M94"), + list(aggregate = "M89", components = c("M91", "M92", "M93")), + list(aggregate = "L47", components = "L94"), + list(aggregate = "L89", components = c("L91", "L92", "L93")) + ) + agg_years_by_code <- DBI::dbGetQuery(con, + "SELECT DISTINCT year, item_code FROM ig_long WHERE is_aggregate") + codes_by_year <- DBI::dbGetQuery(con, "SELECT DISTINCT year, item_code FROM ig_long") + + for (p in pairs) { + agg_years <- agg_years_by_code$year[agg_years_by_code$item_code == p$aggregate] + for (yr in agg_years) { + codes_yr <- codes_by_year$item_code[codes_by_year$year == yr] + has_component <- any(p$components %in% codes_yr) + expect_false( + has_component, + label = sprintf( + "year %s has aggregate-flagged %s co-occurring with a modern component (%s)", + yr, p$aggregate, paste(p$components, collapse = ",") + ) + ) + } + } +}) + +test_that(".verb_spendrev rejects expenditure_concept = 'total' for a non-spending view_base", { + # cog_revenue() never exposes expenditure_concept and always resolves it + # to the "direct" default, so there is no revenue codepath that reaches + # this today -- but .verb_spendrev() is shared, and nothing else stops a + # future caller from passing expenditure_concept = "total" alongside + # view_base = "revenue_annotated", which would UNION expenditure M/L rows + # into a revenue result. Exercise the internal helper directly. + expect_error( + uscogdata:::.verb_spendrev( + verb = "cog_revenue_test", view_base = "revenue_annotated", + subtype_col = "revenue_subtype", + flow_prefixes = c("T", "A", "U", "B", "C", "D"), + call = quote(cog_revenue_test()), + govid = "010000226085", years = 2019L, category = NULL, + per_capita = FALSE, adjust_to_year = NULL, basis = "raw", + recipe = NULL, expenditure_concept = "total" + ), + class = "uscogdata_expenditure_concept_unsupported" + ) +}) diff --git a/tests/testthat/test-views.R b/tests/testthat/test-views.R index 75eaa5b..30391e4 100644 --- a/tests/testthat/test-views.R +++ b/tests/testthat/test-views.R @@ -9,7 +9,9 @@ test_that("all expected views register on session open", { expected <- c( "long", "spending_long", "revenue_long", "canonical_fips_xwalk", "summary_categories", - "spending_annotated", "revenue_annotated" + "spending_annotated", "revenue_annotated", + "ig_long", "ig_annotated", + "ig_long_harmonized", "ig_annotated_harmonized" ) expect_true(all(expected %in% views$table_name)) }) @@ -110,6 +112,70 @@ test_that("inst/sql/22- and 23- harmonized views enforce every WHERE predicate ( expect_equal(rev$amt, 225) }) +test_that("inst/sql/24- and 25- IG views retain aggregates, COALESCE NULL harmonized_code, and exclude the L-- family total (real SQL text, synthetic parquet)", { + # ig_long / ig_long_harmonized have the subtlest predicates in the package: + # a deliberately ABSENT `NOT is_aggregate` (unlike every other *_long view), + # and COALESCE(harmonized_code, item_code) instead of a plain + # `harmonized_code IS NOT NULL` filter. The only end-to-end guard on this + # today is bound to AL state / 2011 / Education K-12, where M12 happens to + # be the sole IG code present -- regenerate the fixture without that one + # row and the guard would die silently while staying green. As with the + # 22-/23- test above, this reads the real inst/sql/24-/25- text off disk + # and executes it against a synthetic hive-partitioned parquet tree, so a + # regression in either predicate changes which rows survive. + skip_if_no_corpus() + + tmp <- withr::local_tempdir() + part_dir <- file.path(tmp, "data", "long", "year=2004") + dir.create(part_dir, recursive = TRUE) + part_path <- file.path(part_dir, "part-0.parquet") + + write_con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(write_con, shutdown = TRUE), add = TRUE) + DBI::dbExecute(write_con, sprintf(" + COPY ( + SELECT * FROM (VALUES + ('ig-A', 'M04', 100, false, 'M04'), -- control: passes through as-is + ('ig-B', 'M38', 50, false, 'M36'), -- fold control: real SB012 rule, renamed to M36 under harmonized basis + ('ig-C', 'M47', 99999, true, NULL), -- legacy aggregate, NO harmonized_code: must survive BOTH views + ('ig-D', 'L--', 55555, false, 'L--'), -- family total: excluded from BOTH views + ('ig-E', 'T29', 44444, false, 'T29') -- wrong prefix (revenue, not M/L): excluded from BOTH views + ) AS t(canonical_govid, item_code, amt, is_aggregate, harmonized_code) + ) TO %s (FORMAT PARQUET) + ", uscogdata:::.sql_lit_chr(part_path))) + + sql_dir <- system.file("sql", package = "uscogdata") + .read_view_sql <- function(filename) { + txt <- paste(readLines(file.path(sql_dir, filename), warn = FALSE), collapse = "\n") + gsub("\\{url\\}", paste0(tmp, "/"), txt, fixed = FALSE) + } + + con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(con, shutdown = TRUE), add = TRUE) + DBI::dbExecute(con, .read_view_sql("10-long.sql")) + DBI::dbExecute(con, .read_view_sql("24-ig_long.sql")) + DBI::dbExecute(con, .read_view_sql("25-ig_long_harmonized.sql")) + + raw <- DBI::dbGetQuery(con, + "SELECT item_code, SUM(amt) AS amt FROM ig_long + GROUP BY item_code ORDER BY item_code" + ) + # L-- (family total) and T29 (wrong prefix) are gone; the aggregate row + # M47 survives -- proof `NOT is_aggregate` is absent from ig_long. + expect_equal(raw$item_code, c("M04", "M38", "M47")) + expect_equal(raw$amt, c(100, 50, 99999)) + + harmonized <- DBI::dbGetQuery(con, + "SELECT item_code, SUM(amt) AS amt FROM ig_long_harmonized + GROUP BY item_code ORDER BY item_code" + ) + # M38 folds to M36 (real harmonized_code present); M47 keeps its raw code + # via COALESCE(NULL, 'M47') -- proof the aggregate row is NOT dropped by + # a plain `harmonized_code IS NOT NULL` filter. L-- and T29 stay excluded. + expect_equal(harmonized$item_code, c("M04", "M36", "M47")) + expect_equal(harmonized$amt, c(100, 50, 99999)) +}) + test_that(".build_series_break_refs matches fin_code + break_year window", { # No series_breaks_pq row falls inside the bundled fixture's 2011-2020 # window (data-verified; see the "series_break_refs" test in @@ -152,6 +218,91 @@ test_that("schema v5 harmonization views register when the corpus supports them" expect_true(all(expected_v5 %in% views$table_name)) }) +test_that(".harmonization_view_files guard is necessary: registration against a v4-shaped corpus (no harmonized_code column at all) succeeds only because the harmonized views are skipped", { + # with_doctored_schema_version() (used elsewhere in this suite) only + # rewrites manifest.json's schema_version -- the underlying `long` parquet + # is still the bundled v6 fixture, which DOES have a harmonized_code + # column, so it only proves the skip *happens*, not that it is *required*. + # This test builds a genuinely v4-shaped corpus: `long` has no + # harmonized_code column at all, matching a real pre-Phase-R2 publish + # tree, and then shows two things: (1) the real .register_views(), gated + # on manifest$schema_version, registers cleanly against it; (2) the exact + # SQL text of a gated file (25-ig_long_harmonized.sql), executed directly + # against the same corpus without the gate, fails -- proving the gate is + # load-bearing, not incidental. + tmp <- withr::local_tempdir() + part_dir <- file.path(tmp, "data", "long", "year=2004") + dir.create(part_dir, recursive = TRUE) + part_path <- file.path(part_dir, "part-0.parquet") + + write_con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(write_con, shutdown = TRUE), add = TRUE) + DBI::dbExecute(write_con, sprintf(" + COPY ( + SELECT * FROM (VALUES + ('gov-1', 'E36', 100, false, 500000, 2020) + ) AS t(canonical_govid, item_code, amt, is_aggregate, population, popyear) + ) TO %s (FORMAT PARQUET) + ", uscogdata:::.sql_lit_chr(part_path))) + + xwalk_path <- file.path(tmp, "data", "canonical_fips_xwalk.parquet") + DBI::dbExecute(write_con, sprintf(" + COPY ( + SELECT * FROM (VALUES + ('gov-1', 'Test Gov', 1, 'County', '01', '001', NULL, 500000) + ) AS t(canonical_govid, gov_name, govs_type, type_label, fips_state, + fips_county, fips_place, population_acs) + ) TO %s (FORMAT PARQUET) + ", uscogdata:::.sql_lit_chr(xwalk_path))) + + cats_path <- file.path(tmp, "data", "summary_categories.parquet") + DBI::dbExecute(write_con, sprintf(" + COPY ( + SELECT * FROM (VALUES + ('E36', 'Test Category', 'expenditure', 'direct', NULL) + ) AS t(item_code, category, category_type, spend_subtype, revenue_subtype) + ) TO %s (FORMAT PARQUET) + ", uscogdata:::.sql_lit_chr(cats_path))) + + # Confirm the synthetic `long` genuinely lacks harmonized_code (not just + # NULL values -- the column itself must be absent) before trusting the + # rest of this test. + cols <- DBI::dbGetQuery(write_con, sprintf( + "DESCRIBE SELECT * FROM read_parquet(%s)", uscogdata:::.sql_lit_chr(part_path) + ))$column_name + expect_false("harmonized_code" %in% cols) + + url <- paste0(tmp, "/") + + # (1) Full .register_views() against this v4-shaped corpus must succeed -- + # this is the behavior the guard exists to protect. + con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(con, shutdown = TRUE), add = TRUE) + expect_no_error( + uscogdata:::.register_views(con, url, manifest = list(schema_version = 4L)) + ) + views <- DBI::dbGetQuery(con, + "SELECT table_name FROM information_schema.tables + WHERE table_schema = 'main' AND table_type = 'VIEW'")$table_name + expect_true(all(c("ig_long", "ig_annotated", "spending_annotated") %in% views)) + expect_false(any(c("ig_long_harmonized", "ig_annotated_harmonized", + "spending_long_harmonized") %in% views)) + + # (2) Prove the gate is load-bearing: the exact SQL text of the skipped + # file, executed directly (bypassing .register_views()'s schema_version + # check) against the SAME corpus, fails because it references + # long.harmonized_code, a column this corpus's `long` does not have. + sql_dir <- system.file("sql", package = "uscogdata") + .read_view_sql <- function(filename) { + txt <- paste(readLines(file.path(sql_dir, filename), warn = FALSE), collapse = "\n") + gsub("\\{url\\}", url, txt, fixed = FALSE) + } + con2 <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(con2, shutdown = TRUE), add = TRUE) + DBI::dbExecute(con2, .read_view_sql("10-long.sql")) + expect_error(DBI::dbExecute(con2, .read_view_sql("25-ig_long_harmonized.sql"))) +}) + test_that("spending_long filters to E/F/G/K prefixes and excludes aggregates", { skip_if_no_corpus() con <- cog_open()