R119: the shared-contract check passes vacuously when a zoom partition is missing, not just empty #200

Open
opened 2026-08-20 21:36:22 +00:00 by art-bot · 0 comments
Collaborator

The shared contracts' R119 check catches an empty partition but not a missing
one, so a country with a ragged zoom ladder passes vacuously.

The gap

Every contract that carries R119 uses the same catalogue query
(conf/shared/contracts/: 02-country_borders, 04-cover, 14-airports,
15-airport_points, 16-buildings, 17-building_points, 18-contours):

SELECT bool_and(pg_relation_size(c.oid) > 0) AS passed,
       count(*) AS partitions,
       string_agg(c.relname, ', ') FILTER (WHERE pg_relation_size(c.oid) = 0) AS empty_partitions
FROM pg_inherits i
JOIN pg_class p ON p.oid = i.inhparent
JOIN pg_class c ON c.oid = i.inhrelid
JOIN pg_namespace n ON n.oid = p.relnamespace AND n.nspname = 'public'
WHERE p.relname ~ '^map_<layer>_[0-9]+$'

count(*) is reported but never asserted. The predicate is bool_and over
whatever partitions exist, so a partition that is absent from pg_inherits
is not "empty" — it is invisible, and the check passes.

R119 as written (docs/country-requirements.md:1516) is "Row counts per zoom per
country, with no unintended empty partition". A missing zoom is squarely the
thing it is meant to catch: an interrupted import, a --layer run that died
between zooms, or a country swapped in at some zooms and not others leaves the
map silently missing a level, and every acceptance run says PASS.

The per-country R119s that predate the folds did assert it — Iceland's old
contours check was count(*) = 5 AND bool_and(...), with a comment saying in as
many words that "count(*) = 5 turns a missing zoom table into a failure rather
than a vacuous pass". The guard was dropped when those checks moved onto the
contracts (#149/#150/#151), because a contract cannot hardcode a number: it does
not know how many countries have a partition.

Suggested shape

It does not need a hardcoded number — the invariant is rectangular: every
country that has any partition of a layer has one at every zoom of that layer.

SELECT bool_and(pg_relation_size(c.oid) > 0)
       AND count(*) = count(DISTINCT p.relname) * count(DISTINCT <cc of c.relname>) AS passed,
       ...

with the offending country/zoom pairs named on failure, the way
empty_partitions names the empty ones. Deriving the country from the partition
name is the fiddly part (right(relname, 2) works today but is not a contract);
pg_get_expr(c.relpartbound, c.oid) is the honest source, or the importer could
grow a helper the way it did for the other catalogue checks.

Notes

  • Not a live failure: a sweep of the current database finds no ragged ladder
    (count(*) = zoom_parents * countries holds for every map_* layer today).
    This is a guard against a class of silent failure, not a fix for a broken
    partition.
  • Seven contracts to update; the wording of R119 in
    docs/country-requirements.md should say the check covers missing as well
    as empty, and the "copy this pair" instruction at :1523 should point at the
    tightened version so the eighth contract inherits it.
  • Found in review of !198 (#151).
The shared contracts' R119 check catches an *empty* partition but not a *missing* one, so a country with a ragged zoom ladder passes vacuously. ## The gap Every contract that carries R119 uses the same catalogue query (`conf/shared/contracts/`: 02-country_borders, 04-cover, 14-airports, 15-airport_points, 16-buildings, 17-building_points, 18-contours): ```sql SELECT bool_and(pg_relation_size(c.oid) > 0) AS passed, count(*) AS partitions, string_agg(c.relname, ', ') FILTER (WHERE pg_relation_size(c.oid) = 0) AS empty_partitions FROM pg_inherits i JOIN pg_class p ON p.oid = i.inhparent JOIN pg_class c ON c.oid = i.inhrelid JOIN pg_namespace n ON n.oid = p.relnamespace AND n.nspname = 'public' WHERE p.relname ~ '^map_<layer>_[0-9]+$' ``` `count(*)` is reported but never asserted. The predicate is `bool_and` over *whatever partitions exist*, so a partition that is absent from `pg_inherits` is not "empty" — it is invisible, and the check passes. R119 as written (docs/country-requirements.md:1516) is "Row counts per zoom per country, with no unintended empty partition". A missing zoom is squarely the thing it is meant to catch: an interrupted import, a `--layer` run that died between zooms, or a country swapped in at some zooms and not others leaves the map silently missing a level, and every acceptance run says PASS. The per-country R119s that predate the folds *did* assert it — Iceland's old contours check was `count(*) = 5 AND bool_and(...)`, with a comment saying in as many words that "count(*) = 5 turns a missing zoom table into a failure rather than a vacuous pass". The guard was dropped when those checks moved onto the contracts (#149/#150/#151), because a contract cannot hardcode a number: it does not know how many countries have a partition. ## Suggested shape It does not need a hardcoded number — the invariant is *rectangular*: every country that has any partition of a layer has one at every zoom of that layer. ```sql SELECT bool_and(pg_relation_size(c.oid) > 0) AND count(*) = count(DISTINCT p.relname) * count(DISTINCT <cc of c.relname>) AS passed, ... ``` with the offending country/zoom pairs named on failure, the way `empty_partitions` names the empty ones. Deriving the country from the partition name is the fiddly part (`right(relname, 2)` works today but is not a contract); `pg_get_expr(c.relpartbound, c.oid)` is the honest source, or the importer could grow a helper the way it did for the other catalogue checks. ## Notes - Not a live failure: a sweep of the current database finds no ragged ladder (`count(*) = zoom_parents * countries` holds for every `map_*` layer today). This is a guard against a class of silent failure, not a fix for a broken partition. - Seven contracts to update; the wording of R119 in `docs/country-requirements.md` should say the check covers *missing* as well as *empty*, and the "copy this pair" instruction at :1523 should point at the tightened version so the eighth contract inherits it. - Found in review of !198 (#151).
art-bot self-assigned this 2026-08-20 21:36:22 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
art/ismap#200
No description provided.