object_reference cannot survive binary pg_upgrade: _sentry_mv crashes it, and repair logic can't fix stale OIDs anyway

[jnasbyupgrade]

Summary

object_reference cannot currently survive a binary pg_upgrade, and the repair logic that's supposed to fix stale OID references after one has its own bugs, independent of that.

Root cause 1: _sentry_mv crashes the upgrade outright

_sentry_mv (CREATE MATERIALIZED VIEW _object_reference._sentry_mv AS SELECT _object_reference._repair();, marked via pg_extension_config_dump()) exists to auto-run _repair() whenever a logical pg_dump/pg_restore happens. Under binary pg_upgrade it fails with:

pg_class heap OID value not set when in binary upgrade mode

Confirmed as a general PostgreSQL limitation, not specific to this extension's code (reproduced with a bare, non-extension materialized view under the same pg_extension_config_dump marking): restoring a config-dumped materialized view triggers a REFRESH, which builds a brand-new heap — and binary-upgrade mode only pre-assigns OIDs for the objects its own schema-restore script explicitly creates, not for a heap created via a later REFRESH.

Root cause 2: real repair work is needed after every binary pg_upgrade — not an edge case

Verified empirically across two independent upgrade pairs (PG 12→13, PG 16→17), with _sentry_mv temporarily worked around purely to let the upgrade complete: of 14 tracked object types spanning nearly every supported cat_tools.object_type, table/index/sequence/view/materialized-view/column/type stay valid (pg_upgrade's OID-preservation covers pg_class/pg_type/pg_enum), but constraint, function, cast, default-value, and trigger — anything backed by a different catalog — get a brand-new OID every single upgrade, unconditionally. This isn't a rare corner case; it's certain to happen on any tracked function/trigger/constraint/cast/default across any binary upgrade.

Root cause 3: the repair functions themselves can't fix that case today

• _repair() (what _sentry_mv invokes) calls _object_oid__add(), which does a plain INSERT INTO _object_oid(...) with no ON CONFLICT. The moment a row already exists for that object_id — exactly what pg_upgrade leaves behind, since it preserves table data physically rather than wiping it the way a logical restore does — it crashes: duplicate key value violates unique constraint "_object_oid_pkey".
• _object_reference.fix_refs()'s "extraneous ID information" branch (the one that should handle "row exists but is stale") references r_object.object_id instead of r_object_v.object_id — a typo that makes it crash (missing FROM-clause entry for table "r_object") instead of emitting its intended warning, for both fix_refs(true) and fix_refs(false).

Net effect

Today: a binary pg_upgrade of a database with object_reference installed fails outright (root cause 1). Even after that's fixed, the extension has no working way to repair the OID staleness that pg_upgrade genuinely introduces for several object types (root causes 2+3) — _repair()/fix_refs() need real fixes to handle "OID row present but stale," not just "OID row missing," regardless of how/when they get invoked.

Also worth fixing while in this area: _object_v__for_update()'s retry loop predates ON CONFLICT

_object_v__for_update()'s current get-or-create for the object row uses a hand-rolled retry loop (SELECT, conditionally INSERT ... ON CONFLICT DO NOTHING, SELECT again, up to 10 times) predating INSERT ... ON CONFLICT support. This can be collapsed to a single INSERT ... ON CONFLICT DO NOTHING RETURNING object_id plus one follow-up SELECT if no row comes back — no loop needed, since ON CONFLICT already waits out a concurrent inserting transaction. Independent of, and simpler than, the _object_oid reconciliation the rest of this issue is about — but the two need to compose cleanly, since _object_v__for_update is a view (joined against _sanity()), so the _object_oid resolution can't be folded into the same RETURNING clause regardless.

Scope note

This issue is about the underlying bugs (the crash, the missing-repair-path bugs, and the ON CONFLICT modernization above) — not about when/how repair gets automatically triggered after an upgrade. That design question is tracked separately in #38.

[jnasbyupgrade]

Claude here. I dug into pg_dump's actual binary-upgrade handling to double-check the "Root cause 1" framing, and it needs a correction: this is not a general PostgreSQL limitation with materialized views under pg_upgrade.

Ordinary materialized views survive binary pg_upgrade fine. pg_dump.c's table-schema dumping code has an explicit special case for this (currently around dumpTableSchema()/the per-table binary-upgrade block):

/*
 * In binary_upgrade mode, restore matviews' populated status by
 * poking pg_class directly.  This is pretty ugly, but we can't use
 * REFRESH MATERIALIZED VIEW since it's possible that some underlying
 * matview is not populated even though this matview is; in any case,
 * we want to transfer the matview's heap storage, not run REFRESH.
 */
if (dopt->binary_upgrade && tbinfo->relkind == RELKIND_MATVIEW &&
    tbinfo->relispopulated)
{
    ...
    "UPDATE pg_catalog.pg_class\n"
    "SET relispopulated = 't'\n"
    "WHERE oid = ..."
}

So for a plain matview, binary upgrade transfers the heap file physically (like any other relation) and just flips relispopulated back on — no REFRESH, no crash.

The actual defect: pg_extension_config_dump() was called on a materialized view, which is outside that function's documented/supported scope. Per extend.sgml's "Extension Configuration Tables" section, it's documented for "a table or a sequence" only — matviews are never mentioned. Nothing stops a script from calling it on a matview's regclass, but pg_dump's implementation has no code path for that: makeTableDataInfo() unconditionally routes any extconfig-marked relation with relkind == RELKIND_MATVIEW to DO_REFRESH_MATVIEW/refreshMatViewData(), i.e. it emits REFRESH MATERIALIZED VIEW ...; as the "data" for that object — with no binary_upgrade exception paralleling the one quoted above. That REFRESH is what tries to build a brand-new heap and hits pg_class heap OID value not set when in binary upgrade mode.

So the crash is a direct, specific consequence of marking _sentry_mv (a matview) as extension-config data — an unsupported combination of two features that individually work fine — not a general incompatibility between materialized views and pg_upgrade.

Practical implication: this reframes Root cause 1 from "unfixable general PostgreSQL limitation, needs a workaround" to "we used pg_extension_config_dump() outside its supported table/sequence scope, and there's likely a different way to achieve the same 'force _repair() to re-run on restore' goal without marking a matview as extconfig."

(Verified directly against current PostgreSQL source: src/bin/pg_dump/pg_dump.c — the binary-upgrade matview relispopulated special case, and makeTableDataInfo()'s unconditional RELKIND_MATVIEW → DO_REFRESH_MATVIEW routing — and doc/src/sgml/extend.sgml's "Extension Configuration Tables" section.)