Re: fastgetattr & isNull - Mailing list pgsql-hackers

From Robert Haas
Subject Re: fastgetattr & isNull
Date
Msg-id 603c8f071001061134k4041d76bx85446927f1c102d0@mail.gmail.com
Whole thread Raw
In response to Re: fastgetattr & isNull  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Wed, Jan 6, 2010 at 1:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.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.

Revised patch attached.  Blow-by-blow:

- fastgetattr() is both a macro and a function.  I previously fixed
the macro; now I've made the function correspond.
- heap_getattr() is always a macro.  The previous version ripped out
the single NULL test therein and this one does the same thing.
- nocachegetattr() already documents that it can't be called when the
attribute being fetched is null, but for some reason it still has an
isNull argument and a bunch of residual cruft, which I have ripped
out, for a substantial improvement in readability, IMHO.
- heap_getsysattr() has an if (isnull) test, which I have removed.
- index_getattr() already follows the proposed coding standard, so no change.
- nocache_index_getattr() is a lobotomized clone of nocachegetattr()
right down to the duplicated comment (gotta love that), and I've given
it the same treatment.
- slot_getattr() already follows the proposed coding standard, so no change.

The naming consistency of these functions and macros leaves a great
deal to be desired.  The arrangement whereby we have a macro called
fetchatt() which calls a macro called fetch_att() is particularly
jaw-dropping.

...Robert

Attachment

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Status of plperl inter-sp calling
Next
From: Robert Haas
Date:
Subject: Re: I: TODO: Allow substring/replace() to get/set bit values