Re: A minor correction in comment in heaptuple.c

On 2014-01-25 16:33:16 -0500, Bruce Momjian wrote:
> On Sat, Jan 25, 2014 at 10:29:36PM +0100, Andres Freund wrote:
> > On 2014-01-25 16:28:09 -0500, Bruce Momjian wrote:
> > > On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
> > > > D'Arcy J.M. Cain <darcy(at)druid(dot)net>
> > > >
> > > > > Although, the more I think about it, the more I think that the comment
> > > > > is both confusing and superfluous.  The code itself is much clearer.
> > > >
> > > > Seriously, if there is any comment there at all, it should be a
> > > > succinct explanation for why we didn't do this (which passes `make
> > > > check-world`):
> > >
> > > Is everyone OK with me applying this patch from Kevin, attached?
> >
> > No. I still think this is stupid. Not at all clearer and possibly breaks
> > stuff.
>
> OK, how about if we change the comment to this:
>
> /*
> --> * assume NULL if attnum is out of range according to the tupdesc
> */
> if (attnum > tupleDesc->natts)
> return true;

I don't think it improves things relevantly, but it doesn't make
anything worse either. So if that makes anybody happy...

I think this style of pinhole copy editing is pretty pointless. There's
dozen checks just like this around. If somebody wants to change the rules
or improve comment it takes more than picking a random one.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services