Re: ALTER TABLE RENAME fix - Mailing list pgsql-patches

On 10 Nov 2001 at 19:08 (-0500), Tom Lane wrote:
| Brent Verner <brent@rcfile.org> writes:
| > | Another problem is that you're assuming that *all* triggers are RI
| > | triggers.  You need to check the trigger function before assuming that
| > | the tgargs are of a form you can parse.  There's at least one place in
| > | the existing code that makes such a check already, but I forget where.
|
| > Yes, I thought about this as I was going to sleep. I believe I can just
| > check that tgisconstraint='t' in the tuple.  I'll make sure this test
| > is sane.
|
| The code I was recalling is in trigger.c, and what it does is to test
| the tgfoid to see if it's one of the RI_FKEY functions:
 [snip]
| I'd suggest sticking with that approach.  (It'd probably be a good idea
| to break this chunk of knowledge out into a function, along the lines of
| bool is_ri_trigger_function(func_oid), rather than duplicating it in two
| different modules.)

Thanks for this tip.  I'll make that code a function.

| > | Also, I would think that you need to look not only at triggers on the
| > | relation containing the att being renamed, but also at triggers on the
| > | rels that are the other side of the FK relationships.  Those triggers
| > | also contain references to the attname, no?
|
| > Either this is already being done, or I do not understand what you mean.
| > The code currently will inspect all tuples where tgrelid or tgconstrrelid
| > is the altered relname's oid.  If relname appears as tgargs[1] or tgargs[2],
| > the attname tgargs[4] or tgargs[5] is updated appropriately.
|
| But your patch has
|
| >   tgrel = heap_openr(TriggerRelationName, AccessShareLock);
| >   ScanKeyEntryInitialize(&skey, 0, Anum_pg_trigger_tgrelid,
| >                      F_OIDEQ, RelationGetRelid(rel));
| >   tgscan = heap_beginscan(tgrel, 0, SnapshotNow, 1, &skey);
|
| The scankey tells the scan subroutines to only return tuples matching
| the scan key, ie, only tuples with tgrelid = OID of the relation.
| So even though you're plowing through all the tuples of pg_trigger,
| only those attached to the target rel will be examined.

Yes, but I'm doing two separate scans for relname's OID; tgrelid and
tgconstrrelid.  The second scan returns tuples where tgrelid is the
OID of the other relname in the constraint.  Is there a way to do
this in one scan?

| Oh, one other thing: patches should be submitted in "diff -c" format.

gotcha.


Thanks,
  Brent

--
"Develop your talent, man, and leave the world something. Records are
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman