#98: check-queries — PREPARE every query plus the PROPERTYNAME cross-check #148

Merged

art merged 2 commits from issue-98-check-queries into main

2026-08-17 22:12:10 +00:00

art commented

2026-08-17 07:14:33 +00:00

Owner

Closes #98

#98: check-queries — PREPARE every query plus the PROPERTYNAME cross-check

What changed

importer check-queries <config> — the guard #98 proposed, landed on the
subcommand shape #129 reserved for it. It catches a layer query reading a
column the source fetch does not request, the failure that shipped once in
#79 and was invisible to review: the broken predicate and the unchanged URL
were a thousand lines apart, and every local signal said fine (the staging
table predated the narrowing, so the query was verified honestly against a
database the config could not reproduce).

Two halves, both read-only (PREPARE + DEALLOCATE, nothing written):

PREPARE guard — every staging query, and every layer source query that
runs against the destination database, is PREPAREd against the live DB and
discarded. Postgres resolves column references without executing, so a
query reading a column its source does not provide fails with 42703 here,
near-free, catching typos in every query at once. Two skip classes, both
the partial-database convention: a query whose table/type is not in this
database yet, and a layer query that reads a source GeoPackage or a foreign
postgres (those run only at import time, against that database). A missing
column is always a failure.
Static PROPERTYNAME cross-check — for every download whose URL carries
a WFS PROPERTYNAME= list (the server honours it strictly), the staging
tables loaded from that file are found, and the columns every query whose
staging: deps include them references are checked against the list. No
database needed, so it is the half that would have failed on the author's
box for #79 — the one whose staging table still carried the dropped
column. References are attributed through FROM/JOIN aliases by a small SQL
scanner (strings/comments/dollar-quotes stripped); unqualified column names
can't be attributed to a table without a full parser and are left to the
PREPARE half.

Also: make check-queries runs the guard on all five configs; the mode table,
a dedicated section, and the subcommand list in docs/importer-cli.md;
AGENTS.md's CLI index.

This is a mechanism-level check, not an SQL acceptance test — it must read the
config's queries and download URLs, which an acceptance_tests: query cannot
reach. Precedent: tiles --check (the other Go walker against the live
database). Not a mechanism gap to file; the same argument that admits
tiles --check admits this.

Verification

make build, make test (importer + mapsrv), go vet, gofmt -l clean.
New unit tests (importer/checkqueries_test.go, no DB): the PROPERTYNAME
parser; the SQL scanner against the real places query (the #79 shape) and
against string/comment/number noise; the cross-check on a synthetic config
reproducing the #79 regression (flagged) and the fixed PROPERTYNAME list
(clean); the load→download wirings (file, databases entry, glob); the
local/remote source split; the CLI dispatch. TestRealConfigsPropertyNameCrossCheck
runs the static half against all five real configs — clean after the #79 fix.

Live (read-only), importer check-queries on all five configs:

check-queries conf/ismap.yaml: 15 query(ies): 7 prepared, 0 failed, 8 skipped, 0 propertyname violation(s)
check-queries conf/semap.yaml: 17 query(ies): 12 prepared, 0 failed, 5 skipped, 0 propertyname violation(s)
check-queries conf/plmap.yaml: 24 query(ies): 24 prepared, 0 failed, 0 skipped, 0 propertyname violation(s)
check-queries conf/fimap.yaml: 16 query(ies): 12 prepared, 0 failed, 4 skipped, 0 propertyname violation(s)
check-queries conf/world.yaml: 2 query(ies): 2 prepared, 0 failed, 0 skipped, 0 propertyname violation(s)

(The skips are layer queries reading source GeoPackages — Sweden's roads
read SverigepaketTP from se_vagdata_tp — and Iceland's roads/water etc.
reading samgongur_linur-style source tables: they run at import time
against the file, not the destination.)

Negative, against the live DB with the staging table present (the author-box
scenario):
- scratch config whose PROPERTYNAME omits RODZAJOBIEKTU while places
  reads g.rodzajobiektu → 1 propertyname violation, exit 1, while the
  PREPARE half still passes (stale table has the column) — exactly the #79
  shape;
- scratch config reading g.bogus_column → PREPARE fails, 42703, exit 1;
- clean scratch config → exit 0.
make acceptance (all five configs, fast bucket): still 0 failed — the
shared dispatch change regresses nothing.
No full import was run; no destructive operations.

Left undone / notes

The static cross-check only attributes qualified references to a table;
an unqualified column reference (SELECT nazwaglowna FROM pl_prng_m1)
is left to the PREPARE half. Fine for the bug class, worth knowing.
The cross-check follows the issue's scope: queries whose staging: deps
declare the table. An undeclared reference to a constrained table is a
DAG bug of its own and is caught by the PREPARE half (against whatever is
staged) rather than the static half.
The #79 "immediate mitigation" from the issue (grep the config for removed
column names when narrowing a fetch) is process advice, not code — the
guard now automates it.

Closes #98 --- #98: check-queries — PREPARE every query plus the PROPERTYNAME cross-check ## What changed `importer check-queries <config>` — the guard #98 proposed, landed on the subcommand shape #129 reserved for it. It catches a layer query reading a column the source fetch does not request, the failure that shipped once in #79 and was invisible to review: the broken predicate and the unchanged URL were a thousand lines apart, and every local signal said fine (the staging table predated the narrowing, so the query was verified honestly against a database the config could not reproduce). Two halves, both read-only (PREPARE + DEALLOCATE, nothing written): 1. **PREPARE guard** — every staging query, and every layer source query that runs against the destination database, is PREPAREd against the live DB and discarded. Postgres resolves column references without executing, so a query reading a column its source does not provide fails with 42703 here, near-free, catching typos in every query at once. Two skip classes, both the partial-database convention: a query whose table/type is not in this database yet, and a layer query that reads a source GeoPackage or a foreign postgres (those run only at import time, against that database). A missing **column** is always a failure. 2. **Static PROPERTYNAME cross-check** — for every download whose URL carries a WFS `PROPERTYNAME=` list (the server honours it strictly), the staging tables loaded from that file are found, and the columns every query whose `staging:` deps include them references are checked against the list. No database needed, so it is the half that would have failed on the author's box for #79 — the one whose staging table still carried the dropped column. References are attributed through FROM/JOIN aliases by a small SQL scanner (strings/comments/dollar-quotes stripped); unqualified column names can't be attributed to a table without a full parser and are left to the PREPARE half. Also: `make check-queries` runs the guard on all five configs; the mode table, a dedicated section, and the subcommand list in `docs/importer-cli.md`; AGENTS.md's CLI index. This is a mechanism-level check, not an SQL acceptance test — it must read the config's queries and download URLs, which an `acceptance_tests:` query cannot reach. Precedent: `tiles --check` (the other Go walker against the live database). Not a mechanism gap to file; the same argument that admits `tiles --check` admits this. ## Verification - `make build`, `make test` (importer + mapsrv), `go vet`, `gofmt -l` clean. - New unit tests (`importer/checkqueries_test.go`, no DB): the PROPERTYNAME parser; the SQL scanner against the real `places` query (the #79 shape) and against string/comment/number noise; the cross-check on a synthetic config reproducing the #79 regression (flagged) and the fixed PROPERTYNAME list (clean); the load→download wirings (file, databases entry, glob); the local/remote source split; the CLI dispatch. `TestRealConfigsPropertyNameCrossCheck` runs the static half against all five real configs — clean after the #79 fix. - Live (read-only), `importer check-queries` on all five configs: ``` check-queries conf/ismap.yaml: 15 query(ies): 7 prepared, 0 failed, 8 skipped, 0 propertyname violation(s) check-queries conf/semap.yaml: 17 query(ies): 12 prepared, 0 failed, 5 skipped, 0 propertyname violation(s) check-queries conf/plmap.yaml: 24 query(ies): 24 prepared, 0 failed, 0 skipped, 0 propertyname violation(s) check-queries conf/fimap.yaml: 16 query(ies): 12 prepared, 0 failed, 4 skipped, 0 propertyname violation(s) check-queries conf/world.yaml: 2 query(ies): 2 prepared, 0 failed, 0 skipped, 0 propertyname violation(s) ``` (The skips are layer queries reading source GeoPackages — Sweden's roads read `SverigepaketTP` from `se_vagdata_tp` — and Iceland's roads/water etc. reading `samgongur_linur`-style source tables: they run at import time against the file, not the destination.) - Negative, against the live DB with the staging table present (the author-box scenario): - scratch config whose PROPERTYNAME omits RODZAJOBIEKTU while `places` reads `g.rodzajobiektu` → **1 propertyname violation**, exit 1, while the PREPARE half still passes (stale table has the column) — exactly the #79 shape; - scratch config reading `g.bogus_column` → **PREPARE fails, 42703**, exit 1; - clean scratch config → exit 0. - `make acceptance` (all five configs, fast bucket): still 0 failed — the shared dispatch change regresses nothing. - No full import was run; no destructive operations. ## Left undone / notes - The static cross-check only attributes *qualified* references to a table; an unqualified column reference (`SELECT nazwaglowna FROM pl_prng_m1`) is left to the PREPARE half. Fine for the bug class, worth knowing. - The cross-check follows the issue's scope: queries whose `staging:` deps declare the table. An *undeclared* reference to a constrained table is a DAG bug of its own and is caught by the PREPARE half (against whatever is staged) rather than the static half. - The #79 "immediate mitigation" from the issue (grep the config for removed column names when narrowing a fetch) is process advice, not code — the guard now automates it.

art added 1 commit

2026-08-17 07:14:33 +00:00

#98 : check-queries — PREPARE every query plus the PROPERTYNAME cross-check 283e89f09b

A layer query can read a column the source fetch does not request, and nothing
catches it until someone imports on a clean machine. It shipped once already
(#79: Poland's `places` read RODZAJOBIEKTU, which a year-old PROPERTYNAME
narrowing had dropped from the fetch; the staging table on the author's box
predated the narrowing, so the query was verified honestly against a database
the config could not reproduce). The guard #98 proposed is now a command, the
`check-queries` subcommand #129 reserved for it. Two halves, both read-only:

1. PREPARE guard — `importer check-queries <config>` PREPAREs each staging
   query and each layer source query that runs against the destination
   database, and discards it. Postgres resolves column references without
   executing, so `column p.rodzajobiektu does not exist` (42703) fails here,
   near-free, and it catches typos in every query at once. A query whose table
   or type is not in this database yet is skipped (the partial-database
   convention), as is a layer query reading a source GeoPackage or a foreign
   postgres — those run only at import time, against that database.

2. Static PROPERTYNAME cross-check — for every download whose URL carries a
   WFS PROPERTYNAME= list (which the server honours strictly), the staging
   tables loaded from that file are found, and the columns every query whose
   `staging:` deps include them references are checked against the list. This
   needs no database at all, so it is the half that fails on the machine that
   introduced the bug — whose staging table still carries the dropped column
   and which the PREPARE guard, validating against whatever is staged now,
   cannot catch.

`make check-queries` runs the guard on all five configs. The check is a
mechanism, not an SQL acceptance test (the `tiles --check` precedent): it must
read the config's queries and download URLs, which an acceptance_tests: query
cannot reach.

Verified: unit tests (the SQL scanner, the cross-check including the exact #79
scenario, dispatch); go vet and gofmt clean; live: all five configs green, a
scratch config repeating the #79 regression fails the static half with the
staging table still present, a bogus column fails PREPARE with 42703, and
`make acceptance` still reports 0 failed.

art changed title from ~~Closes #98~~ to #98: check-queries — PREPARE every query plus the PROPERTYNAME cross-check

2026-08-17 07:14:59 +00:00

art referenced this pull request

2026-08-17 07:15:08 +00:00

Catch a layer query reading a column the source fetch does not request #98

art-bot referenced this pull request

2026-08-17 22:00:50 +00:00

check-queries: cross-check GeoPackage-sourced layer queries against the gpkg schema #155

art-bot added 1 commit

2026-08-17 22:03:16 +00:00

#98 review: unique PREPARE names, honest advice when nothing was checked, and #155 for the gpkg gap ff176b2d82

* A single reused statement name meant one failed DEALLOCATE poisoned the whole
  run: the next PREPARE would hit 42P05 "prepared statement already exists" and
  every remaining query would report that instead of its own verdict. Each query
  gets its own name now, and the DEALLOCATE is best-effort — the statements die
  with the session either way, so a cleanup failure is not a query's verdict and
  is no longer counted as one.

* The "nothing was verified" exit told the reader to import the config's staging
  tables first. That is the right advice for an empty database and the wrong
  advice for a config whose queries all run against a source GeoPackage, where
  no amount of importing will give the PREPARE guard anything to reach. The two
  cases are now distinguished (anyLocalQuery, with a test).

* The GeoPackage gap itself is filed as #155 rather than widened into this PR:
  the two halves are each complete against their own contract, and the gpkg
  schema is a third contract with its own file-format work. It is the largest
  hole — 17 of 74 queries across the five configs skip for this reason and 8 of
  them are Iceland's, most of that country's map — so it is now written down in
  docs/importer-cli.md and in the command's own usage text, where a reader
  meeting a SKIP line will find it. Read a skip as "unchecked", not "fine".

Verified: build, vet, gofmt, go test clean; check-queries still passes all five
configs (74 queries, 57 prepared, 17 skipped, 0 violations) and still catches a
planted bad column on both halves at once (42703 + 1 PROPERTYNAME violation).

art merged commit 1c1df6eddd into main

2026-08-17 22:12:10 +00:00

art deleted branch issue-98-check-queries

2026-08-17 22:12:10 +00:00