Thread
-
update_pg_pwd trigger does not work very well
Tom Lane <tgl@sss.pgh.pa.us> — 2000-02-27T04:48:49Z
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
Bruce Momjian <pgman@candle.pha.pa.us> — 2000-02-27T05:35:42Z
> 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, Pennsylvania 19026
-
Ray Messier <messier@nichewareinc.com> — 2000-02-27T05:52:14Z
help
-
Re: [HACKERS] update_pg_pwd trigger does not work very well
Peter Eisentraut <peter_e@gmx.net> — 2000-02-27T23:54:45Z
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
Bruce Momjian <pgman@candle.pha.pa.us> — 2000-02-28T00:18:22Z
[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, Pennsylvania 19026
-
Re: [HACKERS] update_pg_pwd trigger does not work very well
Jan Wieck <wieck@debis.com> — 2000-02-28T07:34:43Z
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 earlier triggers have already done their external work. I think some "AT COMMIT EXECUTE function(args)" would be better. These functions go into a separate queue and shall not modify any database content any more. At least, it would restrict cases of external data inconsistency to 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 like general 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
Jan Wieck <wieck@debis.com> — 2000-02-28T07:47:21Z
> [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
Tom Lane <tgl@sss.pgh.pa.us> — 2000-02-28T08:02:22Z
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
Jan Wieck <wieck@debis.com> — 2000-02-28T08:13:38Z
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
Don Baccus <dhogaza@pacifier.com> — 2000-02-28T14:16:20Z
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 Service and other goodies at http://donb.photo.net.