Skip to content

Don't clobber caller's client_min_messages on install (#4) - #2

Closed
jnasbyupgrade wants to merge 1 commit into
new_featuresfrom
fix/issue-4-client-min-messages
Closed

Don't clobber caller's client_min_messages on install (#4)#2
jnasbyupgrade wants to merge 1 commit into
new_featuresfrom
fix/issue-4-client-min-messages

Conversation

@jnasbyupgrade

Copy link
Copy Markdown
Owner

Fixes Postgres-Extensions#4.

Problem

The install script began with an unconditional:

SET LOCAL client_min_messages = WARNING;

CREATE EXTENSION runs the entire script in one transaction, so this SET LOCAL applied for the whole statement — including any CASCADE that pulls object_reference in as a dependency. A caller who deliberately set a stricter level (e.g. SET client_min_messages = error to keep install output quiet) was silently overridden, so install-time messages still printed.

Fix

Replace the blind SET with a DO block that only raises the floor to WARNING when the caller's current level is more verbose than WARNING. A stricter level (warning/error) is left untouched. SET LOCAL inside the DO block still applies to the rest of the CREATE EXTENSION transaction (verified) and reverts at commit, so it never leaks into the caller's session.

The level comparison uses array_position over the client_min_messages severity ordering (LOG sorts between DEBUG1 and NOTICE for this GUC).

Verification (PostgreSQL 12)

  • Caller at ERROR: setting preserved through CREATE EXTENSION (previously forced to warning).
  • Caller at NOTICE: install runs quiet at the WARNING floor; caller's NOTICE level restored after commit.
  • Full regress suite: 6/7 unchanged; zzz_build expected output regenerated via the make results copy step — the only diff is deterministic line-number shifts from adding the guard block at the top of the file.

Note on the fleet

This same bug exists across the other extensions in deps/:

  • cat_tools — same SET LOCAL pattern (already tracked as its issue claude-code-review.yml: add missing --comment flag Postgres-Extensions/object_reference#11)
  • count_nulls — uses a plain SET (not LOCAL), so it's worse: it leaks to the whole session, not just the transaction
  • extension_tools — already does a capture/restore, but with plain SET and only on the success path
  • test_factory / pgtap — not affected (use SET LOCAL ROLE / read-only current_setting respectively)

This PR is the reference fix for one project; once approved we'll adapt it to the others (count_nulls needs the SET → guarded SET LOCAL change; cat_tools' edit goes in sql/cat_tools.sql.in).

🤖 Generated with Claude Code

…sions#4)

The install script began with an unconditional
`SET LOCAL client_min_messages = WARNING`. Because CREATE EXTENSION
runs the whole script in a single transaction, that SET LOCAL applied
for the entire statement -- including any CASCADE that pulls
object_reference in as a dependency -- silently overriding a caller
who had deliberately set a stricter level (e.g. `SET client_min_messages
= error`) to keep install output quiet.

Replace it with a DO block that only raises the floor to WARNING when
the caller's current level is more verbose than WARNING; a stricter
level (warning/error) is left untouched. SET LOCAL inside the DO block
still applies to the rest of the CREATE EXTENSION transaction and
reverts at commit, so it never leaks into the caller's session.

Verified on PostgreSQL 12: with the caller at ERROR the setting is
preserved through CREATE EXTENSION; at NOTICE the install runs quiet
and the caller's NOTICE level is restored after commit. The zzz_build
expected output is regenerated via `make results` (line-number shifts
only, from adding the guard block at the top of the file).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jnasbyupgrade

Copy link
Copy Markdown
Owner Author

Closing — opened against the fork by mistake. Reopening against upstream Postgres-Extensions/object_reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant