Re: PATCH: CreateComments: use explicit indexing for ``values''

On Mon, Jun 13, 2011 at 11:09 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>>> Historically this i++ approach has been used in a lot of places that
>>> fill in system catalog tuples.  We've fixed some of them over
>>> time, but I doubt this is the only one remaining.  If we're going
>>> to try to remove it here, maybe we ought to try to fix them all
>>> rather than just this one.
>
> A quick grep reveals that the places that still do it that way are
>
> OperatorShellMake
> OperatorCreate
> TypeShellMake
> TypeCreate
> update_attstats (though this one might be hard to improve)
> CreateComments
> CreateSharedComments
> InsertRule
>
> Of these, all but the two in comment.c follow the convention of
> mentioning the assigned-to column in a comment, so that the code
> is at least somewhat greppable.  So those two definitely need
> improvement, but should we consider changing the others while at it?

Have at it, if you like.

> BTW, there are some contrib modules with functions-returning-record that
> fill in result tuples this way as well.  But we don't have symbolic
> constants for the column numbers there, and it's probably not worth
> introducing such.

Yeah, I think that would not be a good use of time.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company