CI: Document current OID-repair/self-healing behavior

[jnasbyupgrade]

Ground-truth reference for how object_reference's object-identity tracking and OID-repair/self-healing logic behaves today on master: the core model (object/_object_oid tables, _object_v/_object_v__for_update views, _sanity()), every write/repair path in full (object__getsert, _object_oid__add, fix_refs, post_restore, _repair/_sentry_mv, the three event triggers), exhaustive config-dump marking, and three end-to-end scenarios traced with real reproductions in scratch databases: logical pg_dump/pg_restore, a binary pg_upgrade (PG12→PG17), and a stale _object_oid row followed by ALTER ... RENAME.

Also independently confirms, with reproduction, the three known-bug claims referenced in #24: _object_oid__add()'s missing ON CONFLICT, fix_refs()'s r_object/r_object_v typo, and _etg_fix_identity()'s lack of any staleness guard.

Documentation only, describing current behavior — no design proposals. The design question of how/whether to auto-detect and repair after a pg_upgrade remains tracked separately in #24/#25.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[github-actions]

Claude finished @jnasbyupgrade's task in 5m 38s —— View job

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Read sql/object_reference.sql (full source, 1610 lines) and the new doc/oid-repair-current-behavior.md (2803 lines)
• Cross-check every doc section against source (core model, _sanity/views, _object_v__for_update, object__getsert*, fix_refs, post_restore, _repair/_sentry_mv, all three event triggers, config-dump marking, the three end-to-end scenarios)
• Verify the three confirmed-bug claims against source
• Post review feedback

CI is green (31650695158).

Summary

This is a documentation-only PR (single new file, no code changes), and it holds up very well under scrutiny. I verified every "verbatim" SQL quote in the document character-by-character against sql/object_reference.sql, and they all match exactly. The three headline bug claims are all real and correctly diagnosed:

• fix_refs()'s r_object/r_object_v typo — confirmed at sql/object_reference.sql:386 and :393: RAISE WARNING 'extraneous ID information for object_id %', r_object.object_id references an undeclared variable (only r_object_v is declared), so this branch always fails with a plpgsql "record ... is not assigned yet" / undefined-column error instead of producing its intended diagnostic.
• _object_oid__add()/_repair() missing ON CONFLICT — confirmed: the INSERT INTO _object_reference._object_oid(...) at sql/object_reference.sql:296-297 has no conflict clause, so re-running against an already-populated row raises a duplicate-key error. Correctly not idempotent.
• _etg_fix_identity()'s lack of a staleness guard — confirmed: the UPDATE ... FROM _object_reference._object_oid oo at sql/object_reference.sql:1452-1465 blindly trusts the stored (classid, objid, objsubid) and re-derives names from whatever it currently resolves to (or throws an uncaught error if it resolves to nothing), on every ddl_command_end regardless of relevance to the triggering statement.

Also confirmed: the dangling "zzz_object_reference_end" trigger name referenced in the HINT at line 952 doesn't correspond to any of the three actual event triggers (zzz__object_reference_drop, zzz_object_reference__fix_identity, zzz_object_reference_capture) — the doc calls this out correctly as a stale/leftover reference rather than a real missing trigger.

Minor nits (citation-only, don't affect correctness of conclusions)

• _etg_drop() is cited as starting at line 1471; the create_function(...) call actually starts at 1470 (1471 is just the function-name argument).
• A "WHEN NOT r_object_v.ids_ok" branch is mentioned in the _object_oid__add() call-site discussion — no such arm exists in _object_v__for_update's CASE (the real arms are WHEN r_object_v.ids_ok / WHEN NOT r_object_v.ids_exist / WHEN r_object_v.ids_exist). The cited line range (926-940) does point at the correct (real) branch, so this is a wording slip, not a wrong pointer.
• The object__getsert signature citation near the end gives 1168-1174 for a quote that includes loose boolean DEFAULT false, which is actually on line 1175 (off by one).
• The "Files/paths referenced" summary list near the end truncates a couple of line ranges by a few lines relative to the earlier, more careful citations in the body (e.g. giving 1571-1585 for the event-trigger registrations, when the third one runs through 1588).
• One dangling forward-reference ("see the full 53-value enum list captured below") near the end of the document doesn't point at anything in the remaining ~45 lines — likely means "above" instead, referring to content earlier in the doc.

None of these affect the document's substantive conclusions — they're citation-precision slips in an otherwise very carefully sourced and empirically-verified document. No changes are required before merging; the nits above are optional polish.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 50e3fced-fed2-4339-8e11-e07783affcc6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jnasbyupgrade]

Claude here — fixed all five citation nits from the review above in dabe793:

• _etg_drop()'s create_function call now cited as starting at line 1470 (was 1471).
• Removed the fictitious WHEN NOT r_object_v.ids_ok CASE arm from the branch description — only WHEN NOT r_object_v.ids_exist is real for that call site.
• object__getsert() signature range corrected to 1168-1175 (was 1168-1174), to include the loose boolean DEFAULT false line.
• Event-trigger registration range in the "Files/paths referenced" summary corrected to 1571-1588 (was 1571-1585), covering all three CREATE EVENT TRIGGER statements.
• The dangling "captured below" forward-reference to the 53-value enum list corrected to "captured above" (the list appears earlier in the document).

All verified directly against sql/object_reference.sql before fixing, per the review's own note that these are citation-precision slips only — no substantive conclusions changed.

[github-actions]

Claude finished @jnasbyupgrade's task in 1m 15s —— View job

---

Claude finished reviewing this PR

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Read the full diff (doc/oid-repair-current-behavior.md)
• Cross-check documented SQL/behavior claims against sql/object_reference.sql
• Post review feedback

Summary

This PR is unchanged in substance since the last review — the only new commit (dabe793) fixes the five citation nits flagged previously. I re-verified each fix directly against sql/object_reference.sql:

Nit	Fix	Verified
_etg_drop() cited as starting at 1471	Changed to 1470	✅ create_function( call for _etg_drop is at line 1470; '_object_reference._etg_drop' (the name arg) is at 1471
Non-existent WHEN NOT r_object_v.ids_ok branch mentioned	Removed, left only the real WHEN NOT r_object_v.ids_exist	✅ confirmed the CASE at lines 923/926/942 has no ids_ok-negated arm
object__getsert signature cited as 1168-1174	Changed to 1168-1175	✅ loose boolean DEFAULT false is on line 1175
"Files/paths referenced" summary gave 1571-1585 for event trigger registrations	Changed to 1571-1588	✅ the third CREATE EVENT TRIGGER ... zzz_object_reference_capture block runs through line 1588
Dangling "captured below" forward-reference	Changed to "captured above"	✅ matches actual document structure

All five corrections are accurate and no new issues were introduced by this commit — it's a pure precision fix with no changes to the document's substantive content or conclusions.

As covered in the prior review round, the document's core claims (the fix_refs() r_object/r_object_v typo at lines 386/393, the missing ON CONFLICT in _object_oid__add()/_repair(), and _etg_fix_identity()'s lack of a staleness guard) all still check out against current sql/object_reference.sql, and every verbatim SQL quote I spot-checked matches exactly.

No further changes needed — this is ready to merge as far as documentation accuracy goes.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 18s —— View job

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Review the new commit since the last round (af90f20, "Correct pg_upgrade matview claim; rewrite doc for concision")
• Verify the corrected pg_upgrade/matview diagnosis against source
• Check the concision rewrite didn't leave dangling cross-references
• Post review feedback

Summary

This round only adds one new commit, af90f20, on top of the previously-reviewed dabe793. It does two things: trims ~300 lines of redundant re-quoting/scratch-environment narration for concision, and corrects the Scenario (b) pg_upgrade diagnosis.

The correction is a real, substantive improvement, and it checks out. The old text claimed the pg_class heap OID value not set when in binary upgrade mode crash was "a generic populated-matview-vs-binary-upgrade interaction... not specific to object_reference's design." The new text narrows this correctly: it's specific to _sentry_mv being marked via pg_extension_config_dump() (safe_dump()), which I confirmed directly against sql/object_reference.sql:425-426 — _sentry_mv is indeed one of the safe_dump()-marked relations. The doc backs the narrower claim with a differential empirical test (a plain, non-extconfig-marked populated matview survives binary pg_upgrade fine; the extconfig-marked one doesn't), which is good independent evidence even though I can't verify the cited pg_dump.c internals (makeTableDataInfo(), DO_REFRESH_MATVIEW) against actual Postgres source from here.

The concision rewrite didn't break anything I could find. I checked every forward/backward cross-reference the rewrite introduced or left behind (e.g. "presented in full in 'The core model' above," "quoted in full just below" for the _object_v__for_update function) — they all resolve correctly, and no dangling references to the deleted scratch-database/environment-note content remain.

Minor nit (pre-existing, not introduced by this commit — optional polish)

There's a recurring off-by-one in a couple of object__getsert wrapper line citations that predates this PR round and wasn't caught in the earlier citation-nit pass:

• Line ~1741: "the plain wrapper is at lines 1168–1189/1190–1203" — the actual CREATE FUNCTION-equivalent calls run 1167–1188 and 1189–1202 (each citation starts and ends one line late).
• "Files/paths referenced" summary near the end: "1168-1201 (object__getsert wrappers)" has the same one-line-early/late issue (should be ~1167-1202), and "242-269" for the two views should be "242-270" (per the doc's own earlier, correct citation of "256–270" for the second view).

None of this affects the document's conclusions — it's the same class of citation-precision slip flagged and fixed in the previous review round, just in spots that weren't caught then. Not blocking.

No other issues found. Ready to merge as far as documentation accuracy goes.

[github-actions]

Claude finished @jnasbyupgrade's task in 1m 31s —— View job

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Review the new commit since the last round (b5fa86a, "Fix two more citation nits; cut basics-lecture asides")
• Verify the corrected line-number citations against sql/object_reference.sql
• Check the trimmed asides didn't leave a dangling cross-reference
• Post review feedback

Summary

This round adds one commit, b5fa86a, on top of the previously-reviewed af90f20. It's a small, 4-hunk diff: two citation corrections and two prose trims (removing "basics-lecture" asides about Postgres view/function namespace separation).

Both citation corrections are accurate. I re-derived the real line numbers directly from sql/object_reference.sql:

• _object_v view alone is lines 242–255 (not 242–269) — confirmed the view's closing ; is at line 255. Correct fix, since that citation is specifically about _object_v rows, not the paired _object_v__for_update view.
• The combined "_object_v/_object_v__for_update views" range in the "Files/paths referenced" summary is now 242–270 — confirmed _object_v__for_update's closing ; is at line 270. Correct.
• The object__getsert wrapper range is now 1168-1203 — confirmed via grep -n: the two create_function(...) calls run 1168–1189 and 1190–1203 exactly. Correct (this also supersedes a slightly-off guess from the previous review round, which suggested 1167–1202).

One new nit from this commit's trims: line 934 now reads ...distinct from the view of the same name at lines 256–270 — see "Write/repair paths: getsert" above). No section anywhere in the document is titled "Write/repair paths: getsert" (I checked every heading) — this looks like a dangling cross-reference left over from the concision edit. The actual relevant discussion is the paragraph at line 278, under "Main lazy get-or-create / self-heal path..." (§1a "the self-heal engine"), which still explains the view/function name distinction in full. Worth pointing the parenthetical at that section (e.g. "see the self-heal engine section above" or similar) rather than a title that doesn't exist in the doc.

This doesn't affect any substantive conclusion — it's the same class of citation-precision slip caught and fixed in earlier rounds, just introduced by this round's own edit. Not blocking; optional polish before merge.

[github-actions]

Claude finished @jnasbyupgrade's task in 59s —— View job

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Review the new commit since the last round (6830ff9, "Fix dangling cross-reference from previous concision edit")
• Verify the fix against source and prior review notes
• Check for any newly introduced issues
• Post review feedback

Summary

This round adds one commit, 6830ff9, on top of the previously-reviewed b5fa86a. It's a single-line diff fixing the dangling "Write/repair paths: getsert" cross-reference flagged in the last review round — that section title doesn't exist in the document, and the fix correctly repoints it at ### 1a. ... — the self‑heal engine (confirmed at doc/oid-repair-current-behavior.md:280).

However, the fix introduces a new, unbalanced-parenthesis error at line 934. The sentence reads:

... (the plpgsql "getsert" core, lines 835–962; the function, not the view of the same name at lines 256–270 — see the self-heal engine section above (§1a). The call is reached only in the ...

Counting parens in this clause: the outer (the plpgsql "getsert" core, ... opens a parenthetical that is never closed — the only ) before the next sentence belongs to the nested (§1a). The original (now-replaced) text closed the outer paren correctly (... above).), but the replacement adds a second, inner parenthetical (§1a) without adding the second closing ) the nesting now requires. It should read ... section above (§1a)). (or drop the inner parens around §1a entirely, e.g. ... section above, §1a.).

Fix this →

This is purely a prose/formatting nit (unbalanced punctuation, not a factual error — the cross-reference itself now points at the right section) and doesn't affect the document's substantive accuracy. Everything else re-verified from prior rounds still holds: the three headline bug claims, the corrected pg_upgrade/_sentry_mv diagnosis, and all previously-fixed citation corrections remain accurate against sql/object_reference.sql.

Not blocking, but worth a quick one-line fix before merge.

[github-actions]

Claude finished @jnasbyupgrade's task in 41s —— View job

---

Claude finished @jnasbyupgrade's task

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Review the new commit since the last round (524ed2d, "Fix unbalanced parenthesis introduced by previous cross-reference fix")
• Verify the paren fix resolves the issue flagged previously
• Check CI status
• Post review feedback

Summary

This round adds one commit, 524ed2d, on top of the previously-reviewed 6830ff9. It's a single-line fix for the unbalanced-parenthesis nit flagged in the last review round at doc/oid-repair-current-behavior.md:934.

Confirmed fixed. The sentence now reads:

... (the plpgsql "getsert" core, lines 835–962; the function, not the view of the same name at lines 256–270 — see the self-heal engine section above, §1a). The call is reached only in the ...

Instead of adding a second closing paren (the fix I suggested), the commit removed the inner parens around §1a — so there's now exactly one ( (opening the outer parenthetical) and one ) (closing it) in that clause. Parens balance correctly; verified directly against the file at line 934. Either approach resolves the defect; this one reads slightly cleaner.

No other changes in this commit, and no new issues introduced. CI is green (31911497938).

All substantive conclusions from prior rounds still hold: the three headline bug claims (fix_refs()'s r_object/r_object_v typo, _object_oid__add()'s missing ON CONFLICT, _etg_fix_identity()'s lack of a staleness guard), the corrected pg_upgrade/_sentry_mv diagnosis, and all previously-fixed citation corrections remain accurate against sql/object_reference.sql.

Ready to merge.

[github-actions]

Claude finished @jnasbyupgrade's task in 1m 41s —— View job

---

Claude finished @jnasbyupgrade's task

Task list

• Gather context (PR diff, CLAUDE.md, repo structure)
• Review the new commit since the last round (30293eb, "Add a real, standalone Overview section")
• Verify the Overview's claims against sql/object_reference.sql
• Check for dangling cross-references or regressions from earlier rounds
• Post review feedback

Summary

This round adds one commit, 30293eb, on top of the previously-reviewed 524ed2d. It's a pure documentation-structure improvement: replaces the old unlabeled opening prose with an explicit ## Overview section (problem statement, two-table model, object__getsert() behavior, the three staleness scenarios, the known bugs, a net assessment, and a closing pointer marking everything after it as optional supporting material).

Every claim restated in the new Overview checks out against source:

• object__getsert()'s get-or-create-and-reconcile behavior (line 13: missing cache row → auto-recreated; mismatched cache row → hard error, not reconciled) — confirmed directly in _object_v__for_update()'s CASE block at sql/object_reference.sql:919–952: the WHEN NOT r_object_v.ids_exist arm calls _object_oid__add() to fill in a missing row (with a RAISE WARNING first), while WHEN r_object_v.ids_exist (i.e., a mismatching row) unconditionally RAISEs an error.
• The pg_upgrade/_sentry_mv diagnosis restated in bullet 2 — matches, word-for-word in substance, the detailed diagnosis already verified in the prior review round at doc/oid-repair-current-behavior.md:2115–2117 (the pg_extension_config_dump()-on-a-matview root cause, not a general matview/pg_upgrade limitation).
• The _etg_drop() → post_restore() → fix_refs(false) unconditional-repair claim in bullet 3 of the bug list — confirmed: _etg_drop() (sql/object_reference.sql:1470–1515) unconditionally calls object_reference.post_restore() at the end of every invocation, which is SELECT _object_reference.fix_refs(false) (sql/object_reference.sql:409–413). With warning_only = false, every non-OK branch of fix_refs()'s loop (sql/object_reference.sql:333–401) raises a hard error rather than a warning — so a single stale/out-of-sync row anywhere in the tracked set does abort an otherwise-unrelated DROP statement, exactly as claimed.

No new inaccuracies, and no regressions from earlier rounds. The heading structure is unaffected — subsection numbering (### 1., ### 2., ...) resets independently under each ## section, so inserting ## Overview before ## Core Data Model doesn't renumber or collide with anything downstream. All previously-fixed citation nits (the object__getsert wrapper range 1168-1203, the views range 242-270, the _etg_drop line-1470 citation, the self-heal-engine cross-reference, the balanced parenthesis at line 934) remain intact and untouched by this commit.

No issues found. Ready to merge.