[jnasbyupgrade]

Questioned automated-review findings: #44 (comment)

object__getsert() (via _object_v__for_update()) now refuses, with feature_not_supported, to track any object that belongs to the object_reference extension itself. The extension's own members must never become tracked objects.

Part of #40. #45, stacked on this PR, completes #40.

Details

• New _object_reference._is_own_object(classid, objid) checks pg_depend membership (deptype = 'e' against object_reference's own extension row), plus the two objects that are never e members: the extension's declared schema (the extension depends on it, not the other way around) and its own pg_extension row.
• sql/object_reference--0.1.0--stable.sql creates _is_own_object() inside its existing session_replication_role = replica section, and adds the same guard to its copy of _object_v__for_update().
• test/sql/base.sql gains five tests: tracking _object_reference.object, _object_reference._etg_drop, the object_reference schema and the _object_reference schema are each rejected, and _is_own_object() recognizes the extension's own pg_extension row.
• Incidental: _object_v__for_update()'s description now points callers that only need an existing record at _object_reference._object_v.

[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

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 652932b0-541f-4555-988c-612c5c642402

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

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[github-actions]

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

---

Code review of PR #44

I read the diff and found no blocking bugs. I did not run the test suite or CI.

Minor / non-blocking

• _is_own_object() (sql/object_reference.sql, and the copy in sql/object_reference--0.1.0--stable.sql) selects FROM (SELECT ... FROM pg_extension WHERE extname = 'object_reference') e. If that row is missing, the function returns zero rows, so the result is NULL rather than false. The IF guard treats NULL as false, so it works today. Wrapping the body in coalesce(..., false), or restructuring it as a scalar subquery, would make it a true boolean for other callers.
• The _etg_capture body is duplicated in the install script and the update script. The update script is a full create_function replacement, so any later edit to one copy has to be mirrored in the other. The same already holds for _object_v__for_update(), so this is consistent with the existing pattern.
• The hard-coded schema list ('__object_reference', 'object_reference', '_object_reference') appears twice in the _etg_capture filter. It would break if the extension were relocated. That is acceptable, since the extension appears to be non-relocatable.

Looks good

• The new guard raises feature_not_supported before any record is created.
• Skipping the extension's own schemas in _etg_capture, to cover objects not yet recorded as e members during CREATE/ALTER EXTENSION, is well reasoned and documented.
• The special cases for the declared schema and the pg_extension row are correct.
• The test plan count (+3 to +8) matches the five added assertions.

[github-actions]

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

---

Code review of PR #44

I read the diff against origin/master. I did not run the test suite or CI.

Issue: the PR description doesn't match the diff

• The description says _etg_capture now skips the object_reference, _object_reference and __object_reference schemas, in both the install and update scripts.
• The diff has no _etg_capture changes. Commit 8233c1f ("Remove _etg_capture changes; tighten comments") removed them.
• Please update the description so it doesn't claim a guard that isn't there. Check whether this is intended. If _etg_capture isn't changed, an event trigger can still record the extension's own objects during CREATE/ALTER EXTENSION, before they are e members. The PR no longer addresses that case.

Minor / non-blocking

• _is_own_object() returns NULL instead of false if the pg_extension row is missing, because the FROM subquery returns no rows. The IF guard treats NULL as false, so it works today. Wrapping the result in coalesce(..., false) would make it a real boolean for other callers.
• The function body is duplicated in sql/object_reference.sql and sql/object_reference--0.1.0--stable.sql. This matches the existing pattern for _object_v__for_update().
• The new tests (1..17, with the plan +8) cover the tracking table, the event trigger function, both schemas, and the pg_extension row. The pg_extension row is checked by calling _is_own_object() directly, and the test comment explains why.

Looks good

• The guard raises feature_not_supported before any record is created.
• The special cases for the declared schema and the pg_extension row are correct.

[jnasbyupgrade]

Written by Claude.

Replies to the items in the two github-actions reviews (first: #44 (comment), second: #44 (comment)).

_is_own_object() returns NULL instead of false (first review, "Minor": "If that row is missing, the function returns zero rows, so the result is NULL rather than false. [...]"; second review, "Minor": "Wrapping the result in coalesce(..., false) would make it a real boolean for other callers.")

No change. _is_own_object() is itself a member of the object_reference extension, so it can only exist and be called while that extension's pg_extension row exists; CREATE EXTENSION and ALTER EXTENSION UPDATE both have the row in place before the script runs. Its only caller outside the tests is the IF in _object_v__for_update() (checked with git grep -n _is_own_object upstream/refuse-self-tracking -- sql).

_etg_capture duplication and the hard-coded schema list (first review, "Minor", items 2 and 3: "The _etg_capture body is duplicated in the install script and the update script. [...]" and "The hard-coded schema list [...] appears twice in the _etg_capture filter. [...]")

No longer apply. Commit 8233c1f removed this PR's _etg_capture changes; _etg_capture now matches master's.

"the PR description doesn't match the diff" (second review, "Issue")

Fixed; the description has been updated to match the diff.

"If _etg_capture isn't changed, an event trigger can still record the extension's own objects during CREATE/ALTER EXTENSION, before they are e members. The PR no longer addresses that case." (second review)

Not in this PR's scope:

• During CREATE EXTENSION no capture group can be active, because capture groups live in object_reference's own tables.
• For ALTER EXTENSION UPDATE from 0.1.0 with a capture group active in the same transaction, the update does not record the extension's objects. It fails with null value in column "object_names" of relation "object" violates not-null constraint, raised from 0.1.0's _object_reference._etg_fix_identity(), and the transaction rolls back. I verified this by running CREATE EXTENSION object_reference VERSION '0.1.0', object_group__create, capture__start, then ALTER EXTENSION object_reference UPDATE in one transaction.
• That is 0.1.0 code already on master, not something this PR introduces. Add event_trigger__disable()/__enable() for update scripts #45, stacked on this PR, has its update script disable zzz_object_reference_capture and zzz_object_reference__fix_identity at the very start, before the bootstrap statements. I have not tested whether that fixes this case.

_is_own_object duplicated between the install and update scripts (second review, "Minor": "The function body is duplicated in sql/object_reference.sql and sql/object_reference--0.1.0--stable.sql. [...]")

No change. An update script has to carry full replacements for the functions it changes, and, as the review notes, this matches the existing _object_v__for_update() pattern.