'SET LOCAL ROLE blah;' doesn't work?

Lists: pgsql-bugspgsql-hackers
From: Stephen Frost <sfrost(at)snowman(dot)net>
To: pgsql-bugs(at)postgresql(dot)org
Subject: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-28 17:01:44
Message-ID: 20070628170144.GN7531@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

Greetings,

At http://www.postgresql.org/docs/8.2/static/sql-set-role.html, we
claim that 'SESSION and LOCAL modifiers act the same as for the
regular SET command', but I don't think that's actually right...

networx=> set role postgres;
SET
networx*=# show role;
role
----------
postgres
(1 row)

networx*=# reset role;
RESET
networx*=> set session role postgres;
SET
networx*=# show role;
role
----------
postgres
(1 row)

networx*=# reset role;
RESET
networx*=> set local role postgres;
SET
networx*=> show role;
role
------
none
(1 row)

networx*=> reset role;
RESET
networx*=>

Seems like we ignore requests to 'SET LOCAL ROLE blah'. Note that
above it's all in one transaction (indicated by the '*'). Might be
something I'm misunderstanding tho, I suppose... :/ I checked, and
after the 'set local role' I don't seem to have the permissions of
that user (and still have the permissions of my prior user) too.

My inclination is that the documentation is wrong, honestly... I was
originally looking into this area of the documentation to suggest that
we explicitly mention somehow under 'reset all' that 'reset role' isn't
done, and similairly that the documentation under 'set' doesn't apply
for 'set role'.

Thanks,

Stephen


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-28 18:41:28
Message-ID: 17572.1183056088@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> At http://www.postgresql.org/docs/8.2/static/sql-set-role.html, we
> claim that 'SESSION and LOCAL modifiers act the same as for the
> regular SET command', but I don't think that's actually right...

I cannot duplicate the behavior you show, in either HEAD or 8.2.
Are you *sure* you were inside a transaction block?

regards, tom lane


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-28 21:10:37
Message-ID: 20070628211037.GP7531@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > At http://www.postgresql.org/docs/8.2/static/sql-set-role.html, we
> > claim that 'SESSION and LOCAL modifiers act the same as for the
> > regular SET command', but I don't think that's actually right...
>
> I cannot duplicate the behavior you show, in either HEAD or 8.2.
> Are you *sure* you were inside a transaction block?

Wow. Okay, this is more interesting. It has some interaction with:
\set ON_ERROR_ROLLBACK 'on'
in my .psqlrc. If I remove that, then it works as expected. My
understanding of ON_ERROR_ROLLBACK is that it'll set a savepoint before
each command and then will roll back to that savepoint on an error- but
it's still all inside of one main transaction. Seems like
ON_ERROR_ROLLBACK is rolling back on a non-error in this case.. Strikes
me as very bizarre.

Try setting ON_ERROR_ROLLBACK to 'on' and doing 'set local role'...
This is on 8.2.4.

Thanks,

Stephen


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-29 02:06:09
Message-ID: 2925.1183082769@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> I cannot duplicate the behavior you show, in either HEAD or 8.2.

> Wow. Okay, this is more interesting. It has some interaction with:
> \set ON_ERROR_ROLLBACK 'on'
> in my .psqlrc. If I remove that, then it works as expected.

Ah. log_statement = all tells the tale:

regression=# set log_statement='all';
SET
regression=# \set ON_ERROR_ROLLBACK 'on'
regression=# begin;
BEGIN
regression=# set local role tgl;
SET
regression=# show role;
role
------
none
(1 row)

regression=#

and the postmaster log has

2007-06-28 22:02:14.418 EDT 2870 LOG: statement: begin;
2007-06-28 22:02:26.044 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
2007-06-28 22:02:26.045 EDT 2870 LOG: statement: set local role tgl;
2007-06-28 22:02:26.047 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
2007-06-28 22:02:57.545 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
2007-06-28 22:02:57.546 EDT 2870 LOG: statement: show role;
2007-06-28 22:02:57.548 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint

So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
ROLE. Not sure that this is fixable :-(

regards, tom lane


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-29 02:14:01
Message-ID: 20070629021401.GN32626@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

Tom Lane wrote:

> and the postmaster log has
>
> 2007-06-28 22:02:14.418 EDT 2870 LOG: statement: begin;
> 2007-06-28 22:02:26.044 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> 2007-06-28 22:02:26.045 EDT 2870 LOG: statement: set local role tgl;
> 2007-06-28 22:02:26.047 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
> 2007-06-28 22:02:57.545 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> 2007-06-28 22:02:57.546 EDT 2870 LOG: statement: show role;
> 2007-06-28 22:02:57.548 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
>
> So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
> ROLE. Not sure that this is fixable :-(

Maybe if psql sees "SET LOCAL" it shouldn't send the RELEASE command.
But it seems a bit error prone to be finding each command that may be
affected by RELEASE ... what other thing do we have that works at the
level of subtransactions?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-29 02:24:19
Message-ID: 20070629022419.GT7531@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Ah. log_statement = all tells the tale:
[...]
> 2007-06-28 22:02:14.418 EDT 2870 LOG: statement: begin;
> 2007-06-28 22:02:26.044 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> 2007-06-28 22:02:26.045 EDT 2870 LOG: statement: set local role tgl;
> 2007-06-28 22:02:26.047 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
> 2007-06-28 22:02:57.545 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> 2007-06-28 22:02:57.546 EDT 2870 LOG: statement: show role;
> 2007-06-28 22:02:57.548 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint

Hrmpf.

> So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
> ROLE. Not sure that this is fixable :-(

That's not the behavior which is intended though, is it? SET LOCAL
should be for an entire transaction, not for subtransactions/savepoints.
At the very least we should mention this issue in the 'release
savepoint' and 'set local' documentation, and it seems like it'd be very
nice to have fixed, if it can be... :/

Additionally, as I mentioned before, we should really make it clear that
'reset all' doesn't apply to roles. The reason I bring it up is that,
in at least the PG PHP driver, when using persistant connections it's
expected that a 'reset all' handles cleaning things up entirely between
page loads, and that's not the case for roles. I'm not a very trusting
person so I made sure our application always reset the role and was in
general careful but it's something some people might get caught by.
Honestly, it seems likely the driver maintainer will end up adjusting
things to, at least optionally, reset the role explicitly as well.

Of course, alternatively, we could have reset all apply to roles. I
don't honestly feel very strongly either way on that one.

Thanks,

Stephen


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-29 02:28:04
Message-ID: 20070629022804.GU7531@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

* Alvaro Herrera (alvherre(at)commandprompt(dot)com) wrote:
> Tom Lane wrote:
> > 2007-06-28 22:02:14.418 EDT 2870 LOG: statement: begin;
> > 2007-06-28 22:02:26.044 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> > 2007-06-28 22:02:26.045 EDT 2870 LOG: statement: set local role tgl;
> > 2007-06-28 22:02:26.047 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
> > 2007-06-28 22:02:57.545 EDT 2870 LOG: statement: SAVEPOINT pg_psql_temporary_savepoint
> > 2007-06-28 22:02:57.546 EDT 2870 LOG: statement: show role;
> > 2007-06-28 22:02:57.548 EDT 2870 LOG: statement: RELEASE pg_psql_temporary_savepoint
> >
> > So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
> > ROLE. Not sure that this is fixable :-(
>
> Maybe if psql sees "SET LOCAL" it shouldn't send the RELEASE command.
> But it seems a bit error prone to be finding each command that may be
> affected by RELEASE ... what other thing do we have that works at the
> level of subtransactions?

At the very least, anything which does work at the subtransaction level
and not the transaction level should be documented as such... I don't
see anything (perhaps I've missed it) in the 'set local' or the 'release
savepoint' documentation which describes this behavior... :/ Perhaps
I've misunderstood, but generally when we say 'transaction', we're
meaning the a 'full' one and not a savepoint/release in the
documentation. The 'SET LOCAL' documentation even talks explicitly
about COMMIT and ROLLBACK being what causes a reset, RELEASE isn't
mentioned.

Thanks,

Stephen


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-06-29 02:49:20
Message-ID: 3368.1183085360@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
>> So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
>> ROLE. Not sure that this is fixable :-(

> That's not the behavior which is intended though, is it? SET LOCAL
> should be for an entire transaction, not for subtransactions/savepoints.

No, it's supposed to be per subtransaction. Or at least that's the
interpretation we put on it when we made subtransactions; I fear it's
too late to change that now, even if there were a good argument to.

> Additionally, as I mentioned before, we should really make it clear that
> 'reset all' doesn't apply to roles. The reason I bring it up is that,
> in at least the PG PHP driver, when using persistant connections it's
> expected that a 'reset all' handles cleaning things up entirely between
> page loads, and that's not the case for roles.

That assumption is broken anyway. As of 8.3 there's a DISCARD ALL
command that actually does a full cleanup of connection-local state.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-bugs(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 'SET LOCAL ROLE blah;' doesn't work?
Date: 2007-09-09 15:54:35
Message-ID: 24895.1189353275@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs pgsql-hackers

[ see thread at
http://archives.postgresql.org/pgsql-bugs/2007-06/msg00166.php ]

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Alvaro Herrera (alvherre(at)commandprompt(dot)com) wrote:
>> Tom Lane wrote:
>>> So actually, ON_ERROR_ROLLBACK breaks *any* use of SET LOCAL, not just
>>> ROLE. Not sure that this is fixable :-(
>>
>> Maybe if psql sees "SET LOCAL" it shouldn't send the RELEASE command.
>> But it seems a bit error prone to be finding each command that may be
>> affected by RELEASE ... what other thing do we have that works at the
>> level of subtransactions?

> At the very least, anything which does work at the subtransaction level
> and not the transaction level should be documented as such... I don't
> see anything (perhaps I've missed it) in the 'set local' or the 'release
> savepoint' documentation which describes this behavior... :/

I came across this open issue by chance while looking through my mail
folder, and realized that the recently proposed change to SET LOCAL's
behavior would resolve Stephen's complaint. I believe that the end
result of the discussion in this thread:
http://archives.postgresql.org/pgsql-hackers/2007-09/msg00030.php
was that we should make SET LOCAL's effects persist until the end of
the current top transaction, unless reverted by subtransaction rollback
or the save/restore action of a function-local SET option for the same
GUC variable. With that change, psql's automatic RELEASEs for
ON_ERROR_ROLLBACK mode won't affect the state of GUC variables.

So this reinforces my feeling that we came to the right conclusion
in last week's thread. I haven't done anything about revising
the GUC code for that, but will get on it now.

regards, tom lane