Don't clobber caller's client_min_messages on install (#4) - #2
Closed
jnasbyupgrade wants to merge 1 commit into
Closed
Don't clobber caller's client_min_messages on install (#4)#2jnasbyupgrade wants to merge 1 commit into
jnasbyupgrade wants to merge 1 commit into
Conversation
…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>
Owner
Author
|
Closing — opened against the fork by mistake. Reopening against upstream Postgres-Extensions/object_reference. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes Postgres-Extensions#4.
Problem
The install script began with an unconditional:
CREATE EXTENSIONruns the entire script in one transaction, so thisSET LOCALapplied for the whole statement — including anyCASCADEthat pullsobject_referencein as a dependency. A caller who deliberately set a stricter level (e.g.SET client_min_messages = errorto keep install output quiet) was silently overridden, so install-time messages still printed.Fix
Replace the blind
SETwith aDOblock that only raises the floor toWARNINGwhen the caller's current level is more verbose thanWARNING. A stricter level (warning/error) is left untouched.SET LOCALinside theDOblock still applies to the rest of theCREATE EXTENSIONtransaction (verified) and reverts at commit, so it never leaks into the caller's session.The level comparison uses
array_positionover theclient_min_messagesseverity ordering (LOGsorts betweenDEBUG1andNOTICEfor this GUC).Verification (PostgreSQL 12)
ERROR: setting preserved throughCREATE EXTENSION(previously forced towarning).NOTICE: install runs quiet at theWARNINGfloor; caller'sNOTICElevel restored after commit.zzz_buildexpected output regenerated via themake resultscopy 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/:SET LOCALpattern (already tracked as its issue claude-code-review.yml: add missing --comment flag Postgres-Extensions/object_reference#11)SET(notLOCAL), so it's worse: it leaks to the whole session, not just the transactionSETand only on the success pathSET LOCAL ROLE/ read-onlycurrent_settingrespectively)This PR is the reference fix for one project; once approved we'll adapt it to the others (count_nulls needs the
SET→ guardedSET LOCALchange; cat_tools' edit goes insql/cat_tools.sql.in).🤖 Generated with Claude Code