RenameDutchCatalogColumns is live on development and unguarded — it renames Dutch columns the register still declares

[rubenvdlinde]

Status: the step is on development, registered, and unguarded

RenameDutchCatalogColumns merged in #488 (088e14ce, 2026-08-12 07:15:02Z). A blocking review landed on the PR three minutes later and is therefore easy to miss — #488 (comment). This issue restates it against the merged state.

Measured on origin/development after the merge:

repair step registered in appinfo/info.xml	yes
appinfo/info.xml version	0.2.22 — unchanged, not bumped
register still declares naam / beschrijvingKort / publicatiedatum	yes, 19 property keys

The only thing preventing the step from firing is the un-bumped version. Repair steps run on app update and on occ maintenance:repair. The next release bump arms it.

What it will do when it fires

run() renames precisely when the English column is absent (RenameDutchCatalogColumns.php:201) — which is the state of every in-scope schema today, because the rename slice has not landed.

1. sector.naam → sector.name, sector.beschrijving → sector.description, and the same across eleven shard tables (suite, dienst, kwetsbaarheid, organisatie, gebruik, koppeling, beoordeeling, module, bioMaatregel, moduleVersie).
2. The register still declares naam / beschrijving, so on the next schema sync MagicMapper finds naam missing and adds it back, empty.
3. Every read returns null. No error, nothing logged as a failure, invisible to suites that assert against fixtures.

Destinations short_description, publication_date, depublication_date, contact_person are declared nowhere in the register. name/description exist only on element, view, model, relation, property-definition, organization, sbomComponent — none of which is in scope. Extraction positive-controlled at 220 distinct property keys.

This is the step's own header docblock with the two halves swapped, and it targets the ~9,500 imported VNG records the PR names as the population that was never validated.

Nothing enforces the ordering the code requires

appinfo/info.xml states it in prose — "Must run AFTER the register sync that adds the English columns" — with no mechanism behind it. grep -c properties lib/Repair/RenameDutchCatalogColumns.php returns 0: the step never reads a schema's declared properties, so it cannot distinguish "renamed, mapper behind" from "not renamed at all". Those two states are byte-identical in the column list it inspects. The safe back-fill branch fires after the rename; the rename branch fires before, and only the second is currently reachable.

Two ways to close it — either works, "do nothing" does not

1. Guard the step. Gate each rename on the destination being a declared property of that schema. It already resolves the register id and the schema ids from oc_openregister_schemas; reading the declared properties there turns the ordering assumption into a checked precondition and makes the step safe in either merge order. Preferred, because it also survives a future reordering.
2. Land the rename slice before any version bump, and treat the two as one release unit.

The sibling did it a third way that also works: decidesk #467 shipped the property rename and RenameDutchVocabularyColumns in one commit (7a551682), so register and columns move together and the question never arises.

Until one of those lands

Do not bump appinfo/info.xml. That is currently the only thing standing between this step and the outcome above, and a version bump is the most routine change in the repo.

Secondary, worth folding into the same fix

• information_schema is not scoped to the current database. inScopeShardTables() (:298) and columnsOf() (:395) filter only on table_name. On MySQL that view spans every database on the server, so columnsOf() can return another database's column list for a same-named table. Postgres scopes to the current database, which is why the dev rig never showed it. Add AND table_schema = DATABASE() / current_schema().
• $columns is read once per table (:184) and never refreshed after a rename. Safe today only because the collision guard already refuses the one many-to-one family (description). That is load-bearing and undocumented.

[rubenvdlinde]

Decision: guarded. PR #500.

The previous analysis on this issue correctly declined to write the fix, on two grounds. One of them has expired and I closed the other by measuring it.

"The un-bumped version buys real time" — it no longer does. #493, #496, #497, #498 and #499 all merged into this repo today. A hold that depends on nobody touching appinfo/info.xml is not a hold.

"The schemas table's shape was never verified first-hand" — measured. oc_openregister_schemas.properties is a json column, jsonb_typeof = object on all 21 softwarecatalog schemas, keyed by the camelCase property name; properties::jsonb ? 'naam' is true on exactly the schemas the shipped register declares naam on. That is what made the guard writable, and it is why the guard reads the schemas table rather than the register JSON: nothing imports that file automatically, so it describes what an operator may import, not what an install has.

Two things this issue understated

The ordering precondition names an operation nothing performs. info.xml says the step "Must run AFTER the register sync that adds the English columns". No file under lib/ reads the register JSON at all — every reference is a test, a root debug script or a comment (positive control run on a string known to be present). InitializeSettings, ordered immediately before, writes config keys and imports nothing. The register is imported by hand through OpenRegister's UI. So the precondition was not merely unenforced; it was never automated.

The rename branch is not "dangerous in one order" — it is only ever reachable in the wrong state. Once the register moves to English, MagicMapper adds the English column, $new is present, and the back-fill branch runs instead. The rename branch fires precisely when the register has not been renamed.

What it would have done, simulated against a live instance

11 in-scope shard tables, ~35 columns: sector, suite, dienst, kwetsbaarheid, organisatie, gebruik, koppeling, beoordeeling, module, bioMaatregel, moduleVersie. Zero collisions, so the ambiguity guard was never going to stop any of it.

The guard

renameIsSafe($old, $new, $declared) requires both halves — the destination declared and the source no longer declared. Correct in either merge order, a genuine no-op until the register moves, fails closed on a schema whose properties cannot be read.

Proven able to fail: inverting it reddens exactly three tests, by name, with the prediction written first; the other nine stay green. The regression test reads the shipped register file rather than a fixture, and carries two positive controls (36 Dutch columns inspected against a threshold of 20) so an empty loop cannot pass for free.

Deliberately not in that PR

The information_schema-filtered-only-on-table_name issue (MySQL spans every database on the server). Untestable here, and mixing it into a guard on a destructive path is the exact risk the guard exists to remove.

The operational instruction on this issue can be lifted once #500 lands: after it, a version bump is safe, because the step defers instead of renaming.