Thread: update_pg_pwd trigger does not work very well

update_pg_pwd trigger does not work very well

From
Tom Lane
Date:
I was looking at the trigger function that's been added to try to
update pg_pwd automatically if pg_shadow is updated via standard
SQL commands.  It's got some problems:

1. Since the trigger is executed as soon as a tuple is inserted/
updated/deleted, it will write pg_pwd before the transaction is
committed.  If you then abort the transaction, pg_pwd contains wrong
data.  Even if you don't abort, the postmaster may read and act on the
updated pg_pwd before you've committed, which could have bad
consequences (logging in a user who doesn't exist yet, for example).

2. The trigger tries to grab AccessExclusiveLock on pg_shadow.
Since this is being done in the middle of a transaction that has
previously grabbed some lower level of lock on pg_shadow, it's
very easy to create a deadlock situation.  All you need is two
different transactions modifying pg_shadow concurrently, and
it'll fail.

3. CREATE USER and friends refuse to run inside a transaction block
in the vain hope of making life safe for the trigger.  It's vain
since the above problems will occur anyway, if one simply alters
pg_shadow using ordinary SQL commands.  (And if we're not going to
support that, why bother with the trigger?)  I think this is a rather
unpleasant restriction, especially so when it isn't buying any
safety at all.


A possible solution for these problems is to have the trigger procedure
itself do nothing except set a flag variable.  The flag is examined
somewhere in xact.c after successful completion of a transaction,
and if it's set then we run a new transaction cycle in which we
read pg_shadow and write pg_pwd.  (A new transaction is needed so
that it's safe to demand AccessExclusiveLock on pg_shadow --- we
have to release all our old locks before we can do that.)  Note that
*only* this second transaction would need AccessExclusiveLock; CREATE
USER and friends would not.

I am not quite certain that this is completely bulletproof when there
are multiple backends concurrently updating pg_shadow, but I have not
been able to think of a case where it'd fail.  The worst possibility
is that a committed update in pg_shadow might not get propagated to
pg_pwd for a while because some other transaction is holding a lock on
pg_shadow.  (But pg_pwd updates can be delayed for that reason now,
so it's certainly no worse than before.)

Comments?
        regards, tom lane


Re: [HACKERS] update_pg_pwd trigger does not work very well

From
Bruce Momjian
Date:
> I am not quite certain that this is completely bulletproof when there
> are multiple backends concurrently updating pg_shadow, but I have not
> been able to think of a case where it'd fail.  The worst possibility
> is that a committed update in pg_shadow might not get propagated to
> pg_pwd for a while because some other transaction is holding a lock on
> pg_shadow.  (But pg_pwd updates can be delayed for that reason now,
> so it's certainly no worse than before.)
> 
> Comments?

I see your point.  Guess we have to do it inside xact.

--  Bruce Momjian                        |  http://www.op.net/~candle pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


From
"Ray Messier"
Date:
help


Re: [HACKERS] update_pg_pwd trigger does not work very well

From
Peter Eisentraut
Date:
Tom Lane writes:

> 1. Since the trigger is executed as soon as a tuple is inserted/
> updated/deleted, it will write pg_pwd before the transaction is
> committed.  If you then abort the transaction, pg_pwd contains wrong
> data.

Wow, that implies that every trigger that contains non-database
side-effects is potentially bogus. That never occured to me. Perhaps (as a
future plan), it would be a good idea to have deferred triggers as well?
Now that I think of it, wasn't that the very reason Jan had to invent the
separate constraint triggers?

> 2. The trigger tries to grab AccessExclusiveLock on pg_shadow.

It doesn't actually need that exclusive lock, I think. A shared read lock
(i.e., none really) would suffice.

> A possible solution for these problems is to have the trigger procedure
> itself do nothing except set a flag variable.  The flag is examined
> somewhere in xact.c after successful completion of a transaction,
> and if it's set then we run a new transaction cycle in which we
> read pg_shadow and write pg_pwd.

If you think that this is okay (and not just a hack), then go for it. If
the above mentioned deferred triggers are at all in the near future I
wouldn't mind scrapping that trigger altogether. There isn't a good reason
to muck with pg_shadow.{usename|password|validuntil} anyway. And it is in
general not safe to muck with system catalogs period. (Try to rename a
table by updating pg_class.relname. ;)


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden




Re: [HACKERS] update_pg_pwd trigger does not work very well

From
Bruce Momjian
Date:
[Charset ISO-8859-1 unsupported, filtering to ASCII...]
> Tom Lane writes:
> 
> > 1. Since the trigger is executed as soon as a tuple is inserted/
> > updated/deleted, it will write pg_pwd before the transaction is
> > committed.  If you then abort the transaction, pg_pwd contains wrong
> > data.
> 
> Wow, that implies that every trigger that contains non-database
> side-effects is potentially bogus. That never occured to me. Perhaps (as a
> future plan), it would be a good idea to have deferred triggers as well?
> Now that I think of it, wasn't that the very reason Jan had to invent the
> separate constraint triggers?

Yes!  I remember him talking about this.  I bet you can just modify your
trigger to be of that type.


--  Bruce Momjian                        |  http://www.op.net/~candle pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: [HACKERS] update_pg_pwd trigger does not work very well

From
wieck@debis.com (Jan Wieck)
Date:
Peter Eisentraut wrote:

> Tom Lane writes:
>
> > 1. Since the trigger is executed as soon as a tuple is inserted/
> > updated/deleted, it will write pg_pwd before the transaction is
> > committed.  If you then abort the transaction, pg_pwd contains wrong
> > data.
>
> Wow, that implies that every trigger that contains non-database
> side-effects is potentially bogus. That never occured to me. Perhaps (as a
> future plan), it would be a good idea to have deferred triggers as well?
> Now that I think of it, wasn't that the very reason Jan had to invent the
> separate constraint triggers?
   The  reason  for  the trigger queue was deferrability at all.   And the way it's implemented doesn't help out.
   Imagine that the trigger not only affects external data,  but   does  further  checks  and/or modifications to table
data.If   some statement affects multiple rows, a trigger ran from  the   queue  can  still abort the transaction, but
earliertriggers   have already done their external work.
 
   I think some "AT  COMMIT  EXECUTE  function(args)"  would  be   better.   These  functions go into a separate queue
andshall   not modify any database content any more. At least, it  would   restrict cases of external data
inconsistencyto cases, where   external modifications fail.
 

> > itself do nothing except set a flag variable.  The flag is examined
> > somewhere in xact.c after successful completion of a transaction,
> > and if it's set then we run a new transaction cycle in which we
> > read pg_shadow and write pg_pwd.
>
> If you think that this is okay (and not just a hack), then go for it.
   I consider it a hack, since this particular trigger  needs  a   global flag known explicitly by xact routines. I
likegeneral   solutions instead.
 


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#========================================= wieck@debis.com (Jan Wieck) #




Re: [HACKERS] update_pg_pwd trigger does not work very well

From
wieck@debis.com (Jan Wieck)
Date:
> [Charset ISO-8859-1 unsupported, filtering to ASCII...]
> > Tom Lane writes:
> >
> > > 1. Since the trigger is executed as soon as a tuple is inserted/
> > > updated/deleted, it will write pg_pwd before the transaction is
> > > committed.  If you then abort the transaction, pg_pwd contains wrong
> > > data.
> >
> > Wow, that implies that every trigger that contains non-database
> > side-effects is potentially bogus. That never occured to me. Perhaps (as a
> > future plan), it would be a good idea to have deferred triggers as well?
> > Now that I think of it, wasn't that the very reason Jan had to invent the
> > separate constraint triggers?
>
> Yes!  I remember him talking about this.  I bet you can just modify your
> trigger to be of that type.
   He  could make the trigger look like a by default deferred RI   trigger in pg_trigger, of course. Then it will  go
onto the   queue.
 
   But as soon as someone does
       SET CONSTRAINTS ALL IMMEDIATE;
   it  will be fired if queued or as soon as it appears. So it's   not the final solution.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#========================================= wieck@debis.com (Jan Wieck) #




Re: [HACKERS] update_pg_pwd trigger does not work very well

From
Tom Lane
Date:
wieck@debis.com (Jan Wieck) writes:
>>>> itself do nothing except set a flag variable.  The flag is examined
>>>> somewhere in xact.c after successful completion of a transaction,
>>>> and if it's set then we run a new transaction cycle in which we
>>>> read pg_shadow and write pg_pwd.
>> 
>> If you think that this is okay (and not just a hack), then go for it.

>     I consider it a hack, since this particular trigger  needs  a
>     global flag known explicitly by xact routines. I like general
>     solutions instead.

Well, really it's pg_pwd itself that is a hack --- we wouldn't need
to be worrying about all this if pg_pwd didn't exist outside the
database/transaction universe.  But I don't think it'd be wise to
try to bring the postmaster into that universe, so we're stuck with
a hack for exporting user authorization info.

If we had examples of other problems that could be solved by such
a mechanism, then I'd agree with Jan that we ought to invent a general
after-commit-do mechanism.  But I don't recall users clamoring for it,
so I question whether the extra effort is worthwhile.
        regards, tom lane


Re: [HACKERS] update_pg_pwd trigger does not work very well

From
wieck@debis.com (Jan Wieck)
Date:
Tom Lane wrote:

> wieck@debis.com (Jan Wieck) writes:
> >     I consider it a hack, since this particular trigger  needs  a
> >     global flag known explicitly by xact routines. I like general
> >     solutions instead.
>
> Well, really it's pg_pwd itself that is a hack --- we wouldn't need
> to be worrying about all this if pg_pwd didn't exist outside the
> database/transaction universe.  But I don't think it'd be wise to
> try to bring the postmaster into that universe, so we're stuck with
> a hack for exporting user authorization info.
>
> If we had examples of other problems that could be solved by such
> a mechanism, then I'd agree with Jan that we ought to invent a general
> after-commit-do mechanism.  But I don't recall users clamoring for it,
> so I question whether the extra effort is worthwhile.
   Exactly  these  days  there  was  someone  having  trouble to   dynamically load the tclLDAP package into PL/Tcl.
He wanted   to UPDATE his LDAP from inside a trigger.
 
   If  he hasn't had this loading problem, I'd never known. So I   assume there are already constructs like this out
there.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#========================================= wieck@debis.com (Jan Wieck) #




Re: [HACKERS] update_pg_pwd trigger does not work very well

From
Don Baccus
Date:
At 08:34 AM 2/28/00 +0100, Jan Wieck wrote:

>    I consider it a hack, since this particular trigger  needs  a
>    global flag known explicitly by xact routines. I like general
>    solutions instead.

It's very much a hack.  Something like the "on commit execute()" 
solution would be much better.  How crucial is a short-term fix
for this particular problem?  Could it wait until a more generalized
facility is provided?

If not I suppose it could be hacked in with copious notes that it
should be redone later.



- Don Baccus, Portland OR <dhogaza@pacifier.com> Nature photos, on-line guides, Pacific Northwest Rare Bird Alert
Serviceand other goodies at http://donb.photo.net.