Re: fastgetattr & isNull

On Wed, Jan 6, 2010 at 1:16 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Spoke with Bruce on IM and we think the best option is to just remove
>> the NULL tests.  Since it's been this way for 11 years, presumably
>> nobody is trying to use it with a NULL fourth argument.
>
>> Proposed patch attached.
>
> There are a number of is-null checks in related code that ought to go
> away too --- look at heap_getattr, nocachegetattr, etc.  Our principle
> here ought to be that none of the field-fetching routines allow a null
> pointer.

I was just noticing this in the non-macro version of fastgetattr().
Let me go take a look at that.

> I wouldn't bother with those added comments.  They wouldn't have been
> there if the code had always been like this.  If you feel a need to
> have a comment, it should be more like "Before Postgres 8.5, the isnull
> argument could be a null pointer, but we no longer allow that".  That
> way tells people that there was a change here that might affect their
> code, whereas the addition you suggest wouldn't flag that.

Well, that comment is a bit misleading too, since a pointer with a
NULL value might work but a literal NULL certainly doesn't.

...Robert