pgsql: Recalculate search_path after ALTER ROLE.

Started by Jeff Davisalmost 3 years ago4 messagescomitters
Jump to latest
#1Jeff Davis
pgsql@j-davis.com

Recalculate search_path after ALTER ROLE.

Renaming a role can affect the meaning of the special string $user, so
must cause search_path to be recalculated.

Discussion: /messages/by-id/186761d32c0255debbdf50b6310b581b9c973e6c.camel@j-davis.com
Reviewed-by: Nathan Bossart, Michael Paquier
Backpatch-through: 11

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/fa2e874946c5b9f23394358c131e987df7cc8ffb

Modified Files
--------------
src/backend/catalog/namespace.c | 6 +-
src/test/isolation/expected/search-path-inval.out | 97 +++++++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/search-path-inval.spec | 59 ++++++++++++++
4 files changed, 162 insertions(+), 1 deletion(-)

#2Jeff Davis
pgsql@j-davis.com
In reply to: Jeff Davis (#1)
Re: pgsql: Recalculate search_path after ALTER ROLE.

On Wed, 2023-08-09 at 20:31 +0000, Jeff Davis wrote:

Recalculate search_path after ALTER ROLE.

Looks like this is causing some buildfarm failures for
debug_parallel_query members related to the combination of parallel
query, search_path, and renaming a role.

I'm not sure yet, but this looks like a possible pre-existing bug, or
some incorrect assumption I made in fa2e874946.

Investigating...

Regards,
Jeff Davis

#3Jeff Davis
pgsql@j-davis.com
In reply to: Jeff Davis (#2)
Re: pgsql: Recalculate search_path after ALTER ROLE.

On Wed, 2023-08-09 at 16:12 -0700, Jeff Davis wrote:

I'm not sure yet, but this looks like a possible pre-existing bug, or
some incorrect assumption I made in fa2e874946.

Before my commit the behavior is the same. Here's a minimal repro
(debug_parallel_query set to 'on' or 'regress'):

s1:
CREATE USER u1;
SET SESSION AUTHORIZATION u1;

s2:
ALTER ROLE u1 RENAME TO u2;

s1:
SELECT 1;
ERROR: role "u1" does not exist
CONTEXT: while setting parameter "session_authorization" to "u1"

Given that it works fine without parallel query, I'm inclined to call
this a parallel query bug. The error is coming from RestoreGUCState().

That being said, the buildfarm is red after my commit, so unless
someone sees a quick fix here, perhaps I should remove my isolation
test? I don't want to revert my whole commit, because it seems correct
and fixes a separate bug.

Regards,
Jeff Davis

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jeff Davis (#3)
Re: pgsql: Recalculate search_path after ALTER ROLE.

Jeff Davis <pgsql@j-davis.com> writes:

On Wed, 2023-08-09 at 16:12 -0700, Jeff Davis wrote:

I'm not sure yet, but this looks like a possible pre-existing bug, or
some incorrect assumption I made in fa2e874946.

Before my commit the behavior is the same. Here's a minimal repro
...
Given that it works fine without parallel query, I'm inclined to call
this a parallel query bug. The error is coming from RestoreGUCState().

Check. Seems like we need a way to restore the role GUC without
performing a catalog lookup. I'll bet there are comparable bugs
with search path, temp tablespace names, etc.

That being said, the buildfarm is red after my commit, so unless
someone sees a quick fix here, perhaps I should remove my isolation
test? I don't want to revert my whole commit, because it seems correct
and fixes a separate bug.

Yes, just remove the test case for now. It might or might not be
worth the trouble to put it back later.

regards, tom lane