Re: ALTER TABLE RENAME fix

Brent Verner <brent@rcfile.org> writes:
> These patches fix the problem where an
>   ALTER TABLE <table> RENAME <oldcolumn> TO <newcolumn>
> did not update the RI_ triggers if the oldcolumn was referenced in
> a RI constraint.

This seems wrong on its face: the code unconditionally updates the
attname part of a trigger's tgargs without checking whether the trigger
actually refers to the attribute being renamed.  Don't you need to have
a comparison against the old attname in there somewhere?

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.

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?

A performance issue here is that scanning all of pg_trigger could be
kind of slow in a database with lots and lots of triggers.  You should
be doing an indexscan using pg_trigger_tgrelid_index.  Actually, given
the previous comment, probably more than one indexscan.  My inclination
would be to accumulate the distinct tgconstrrelid values seen on
triggers that are found to refer to the renamed att during the first
indexscan on the original rel's relid, and then do additional indexscans
looking at those relids (probably there won't be many).

One stylistic quibble: the direct references to CurrentMemoryContext
are verbose and unnecessary.  Use palloc() and pstrdup().

            regards, tom lane