Thread

1. Re: ALTER TABLE

   Tom Lane <tgl@sss.pgh.pa.us> — 2000-08-03T17:03:55Z

   Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
   >> Yes, this is a known bug.  Two bugs actually, first being that the
   >> trigger definitions don't track the rename (they should probably be
   >> storing OID not relname, although that would complicate dump/restore).
   
   > I do plan on trying to move all the table/attribute storage to OIDs. 
   > I actually don't think it'll be too hard on dump/restore, since they
   > should be able to get rewritten as an ALTER TABLE ADD CONSTRAINT
   > rather than the CREATE CONSTRAINT TRIGGER, so it should just be
   > a matter of turning the oids back into rel/attrib names at dump time.
   
   That's doable, certainly, but I think it will be a little bit fragile.
   How will pg_dump know which arguments of which triggers need to be
   processed in this fashion?  Some ugly hardwired assumptions will be
   needed AFAICS.
   
   I think this ties into the discussions we've had on-and-off about not
   storing enough metadata.  It'd be better if the FK constraints were
   stored explicitly in some system table or other, in a form designed
   for inspection, and not solely stored in a form designed for execution.
   It's the same kind of problem we have with SERIAL columns...
   
   			regards, tom lane
   
   

2. Re: ALTER TABLE

   Tom Lane <tgl@sss.pgh.pa.us> — 2000-08-03T19:33:37Z

   Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
   > I believe these are all the cases I saw of heap_openr with no lock
   > and no check in the actual trigger functions (patch to
   > hopefully elog(ERROR) instead of crashing attached).
   > [ patch snipped ]
   
   We had a discussion about that on 11-July and the consensus seemed to be
   that the real problem is heap_open's definition; it's too easy to forget
   when you need to check for failure return.  I have just committed
   changes that split heap_open into two routines: heap_open() now ALWAYS
   elogs on failure, regardless of lock mode, while heap_open_nofail() is
   what to call if you really want a NULL return on failure.  (Likewise for
   heap_openr() of course.)  I found only about three places in the whole
   backend that really wanted the _nofail() case.
   
   Accordingly, this patch is not needed anymore in current sources, though
   it'd still be the most convenient fix for 7.0.* series if anyone is
   concerned enough to apply it.
   
   A possibly more important issue: why are the RI triggers opening the
   referenced rel with NoLock anyway?  Doesn't that leave you open to
   someone deleting the referenced rel out from under you while you are
   working with it?  Seems like at minimum you should grab AccessShareLock.
   
   			regards, tom lane
   
   

3. Re: [BUGS] ALTER TABLE

   Stephan Szabo <sszabo@megazone23.bigpanda.com> — 2000-08-03T20:08:56Z

   On Thu, 3 Aug 2000, Tom Lane wrote:
   
   > Accordingly, this patch is not needed anymore in current sources, though
   > it'd still be the most convenient fix for 7.0.* series if anyone is
   > concerned enough to apply it.
   Yeah, actually, a friend of mine ran into this recently with incorrect 
   create constraint trigger statements so I already was going to send a
   patch to him, then it got mentioned on -bugs.
   
   > A possibly more important issue: why are the RI triggers opening the
   > referenced rel with NoLock anyway?  Doesn't that leave you open to
   > someone deleting the referenced rel out from under you while you are
   > working with it?  Seems like at minimum you should grab AccessShareLock.
   That's a good point.  To be honest, I don't really know why it's not
   grabbing a lock (Jan?).  As a general newbie question for such things,
   what happens to your relation pointer if it were to be deleted out
   from under?  I figure that if it gets to the actual query, it will fail
   (unless someone were to create a table with that name in the meantime -
   ouch...)