Re: [WIP] In-place upgrade

> We already do check the page version on read-in --- see PageHeaderIsValid.

Right, but the only place this is called is in ReadBuffer_common,
which doesn't seem like a suitable place to deal with the possibility
of a V3 page since you don't yet know what you plan to do with it.
I'm not quite sure what the right solution to that problem is...

>> But I think we probably need some input from -core on this topic as well.
> I concur that I don't want to see this patch adding more than the
> absolute unavoidable minimum of overhead for data that meets the
> "current" layout definition.  I'm disturbed by the proposal to stick
> overhead into tuple header access, for example.

...but it seems like we both agree that conditionalizing heap tuple
header access on page version is not the right answer.  Based on that,
I'm going to move the "htup and bufpage API clean up" patch to
"Returned with feedback" and continue reviewing the remainder of these
patches.

As I'm looking at this, I'm realizing another problem - there is a lot
of code that looks like this:

void HeapTupleSetXmax(HeapTuple tuple, TransactionId xmax)
{     switch(tuple->t_ver)     {             case 4 : tuple->t_data->t_choice.t_heap.t_xmax = xmax;
        break;             case 3 : TPH03(tuple)->t_choice.t_heap.t_xmax = xmax;                              break;
        default: elog(PANIC, "HeapTupleSetXmax is not supported.");     }
 
}

TPH03 is a macro that is casting tuple->t_data to HeapTupleHeader_03.
Unless I'm missing something, that means that given an arbitrary
pointer to HeapTuple, there is absolutely no guarantee that
tuple->t_data->t_choice actually points to that field at all.  It will
if tuple->t_ver happens to be 4 OR if HeapTupleHeader and
HeapTupleHeader_03 happen to agree on where t_choice is; otherwise it
points to some other member of HeapTupleHeader_03, or off the end of
the structure.  To me that seems unacceptably fragile, because it
means the compiler can't warn us that we're using a pointer
inappropriately.  If we truly want to be safe here then we need to
create an opaque HeapTupleHeader structure that contains only those
elements that HeapTupleHeader_03 and HeapTupleHeader_04 have in
common, and cast BOTH of them after checking the version.  That way if
somone writes a function that attempts to deference a HeapTupleHeader
without going through the API, it will fail to compile rather than
mostly working but possibly failing on a V3 page.

...Robert