Remove redundant client_min_messages handling from install script

[jnasbyupgrade]

CREATE EXTENSION already raises client_min_messages to WARNING for the install script (only-raising, so a stricter caller is respected) and restores it afterward — so the script's own setting was redundant, and being blind it actually lowered a stricter caller's level during install. Remove it.

Closes #4

[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: 947a6144-a4e7-4994-98c5-f5c288eec00f

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 please review this PR.

[claude]

Claude finished @jnasbyupgrade's task in 4m 37s —— View job

---

Review: sql/object_reference.sql — Don't clobber caller's client_min_messages on install

• Gather context (PR body, diff, comments)
• Review sql/object_reference.sql changes
• Check zzz_build test impact story
• Post review feedback

Summary

The fix is logically sound. I traced through the GUC/transaction semantics carefully:

• Transaction scoping is correct. SET (without LOCAL) inside the DO block is still a transactional GUC change — if CREATE EXTENSION aborts partway through, Postgres unwinds it along with everything else, so the "rolled back automatically if the install fails" claim in the comment (sql/object_reference.sql:7) holds.
• Using plain SET + an explicit restore at the bottom, rather than SET LOCAL, is the right call, not just an equivalent option. SET LOCAL would hold the raised WARNING floor until the enclosing transaction commits — which, under CREATE EXTENSION ... CASCADE, would also suppress a downstream extension's own NOTICEs after object_reference finishes installing. The explicit restore at sql/object_reference.sql:1633-1638 narrows the suppression window to just this script's own statements. Worth noting: the PR description says "SET LOCAL inside the DO block still applies to the rest of the CREATE EXTENSION transaction and reverts at commit" — that describes a SET LOCAL-based approach, but the actual code uses plain SET with a manual restore, which behaves differently (and better, for the CASCADE case) than what's described. Might be worth tightening the PR description so future readers aren't misled about which mechanism is in play.
• The LOG ordering nuance is correct. For client_min_messages specifically, LOG does sort between DEBUG1 and NOTICE (unlike log_min_messages, where LOG sorts near the top), and c_levels reflects that correctly.
• Fail-safe edge case: c_levels doesn't include fatal/panic. If a caller had client_min_messages set to one of those, array_position returns NULL for v_current's position, the IF condition evaluates to NULL (treated as false), and the floor is correctly not raised — so an unusually strict caller setting is preserved even though it's not explicitly handled. Accidental, but correct.

Minor nit

v_current (sql/object_reference.sql:18) is declared CONSTANT but uses the v_ prefix. The rest of the file's convention reserves c_ for CONSTANT-qualified declarations (c_levels two lines above it, plus c_schema, c_oid_field, c_classid, c_catalog elsewhere in the file). Purely cosmetic — consider renaming to c_current for consistency.

Fix this →

Outstanding action item (already flagged by the author)

test/expected/zzz_build.out embeds source line numbers from the install script (psql:test/temp_load.not_sql:188, 189, 513, 620, 627 — I confirmed these five references directly in the current expected-output file). Since this PR adds 32 lines, those all need to shift by +32 before this test will pass. The author was upfront that they couldn't regenerate this locally (build failure with cat_tools 0.2.1 on modern Postgres, unrelated to this change) and that it needs regenerating via make results in the canonical CI environment. I checked CI status for this PR and no workflow runs have been reported yet, so this is still unverified — worth confirming the zzz_build test actually goes green before merging.

Overall: the core logic is correct and more carefully considered than the PR description gives it credit for (the plain-SET-plus-manual-restore approach is deliberately narrower-scoped than SET LOCAL, which is the right call for the CASCADE case). Only a cosmetic naming nit, plus the known/disclosed zzz_build expected-output regeneration still needs to land and go green in CI.