Re: commands subdirectory continued -code cleanup

On Fri, 2002-04-19 at 20:34, Tom Lane wrote:
> John Gray <jgray(at)azuli(dot)co(dot)uk> writes:
> > Sequences still seem to work after they've had attributes renamed, but I
> > see little value in being able to do this. Is it OK to prohibit the
> > renaming of sequence columns?
>
> That seems like an error to me. Setting defaults, constraints, etc on a
> sequence is bogus too --- do we catch those?
>
Yes, we catch those. The current gaps in checking are:
1) renameatt allows any type of relation to have columns renamed
(including indexes, but that case is caught by heap_open objecting to
the relation being an index)

2) add/drop default allows the addition/dropping of constraints on
system attributes (Apropos of this, I think it might also be good to add
a check into AddRelationRawConstraints that verifies that attnum in
colDef is actually positive and <natts for the relation -just in case it
is passed a bogus structure from somewhere else. This would be a cheap
check to do.)

> Not sure. There are subroutines in utility.c that are useful for
> this purpose, and I don't really see the value of having them called
> from all over the place when it can be more localized. We should
> probably be consistent about having tablecmds.c make all the relevant
> permissions checks for its operations, but I don't think that
> necessarily translates into the same choice for the rest of commands/.
> AFAIR none of the rest of commands/ has the recursive-operations issue
> that forces this approach for table commands.
>

OK. I can remove the duplicate system relation check from renamerel and
just leave a comment explaining that the permissions checks are
performed in utility.c. (renamerel is unlike other tablecmds.c functions
in that it doesn't recurse. It is also called from cluster.c but again,
permissions are checked for that in tcop/utility.c)

Regards

John