Re: More heap tuple header fixes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Manfred Koizar <mkoi-pg(at)aon(dot)at>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: More heap tuple header fixes
Date: 2002-07-20 20:27:14
Message-ID: 14202.1027196834@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Manfred Koizar <mkoi-pg(at)aon(dot)at> writes:

> + tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
> HeapTupleHeaderSetXmin(tup->t_data, GetCurrentTransactionId());
> HeapTupleHeaderSetCmin(tup->t_data, cid);
> HeapTupleHeaderSetXmaxInvalid(tup->t_data);
> - HeapTupleHeaderSetCmax(tup->t_data, FirstCommandId);
> - tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
> + /*
> + * Do *not* set Cmax! This would overwrite Cmin.
> + */
> + /* HeapTupleHeaderSetCmax(tup->t_data, FirstCommandId); */
> tup->t_data->t_infomask |= HEAP_XMAX_INVALID;

This sort of thing crystallizes the vague unease I had about those
HeapTupleHeader macros.

I'd recommend redesigning the HeapTupleHeaderSet macros so that they
do not do any setting of t_infomask bits, or even take any conditional
action based on them, but solely Assert() that the bits are already
in the appropriate state to allow storing of the value to be stored.
Then, all uses have to be checked to ensure that t_infomask is coerced
into the right state *before* doing HeapTupleHeaderSetFoo. Anything
else is subject to order-of-operations mistakes that were not errors
before, and cannot be detected by the macros as now defined. The
cmax-set-is-not-okay bug illustrated above is a perfect example of
what I'm talking about.

regards, tom lane

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Rijndael AES Cipher 2002-07-20 20:55:12 NLS translation for libpq(.so) - pt_BR
Previous Message Tom Lane 2002-07-20 19:26:02 Re: pgsql/ oc/src/sgml/release.sgml rc/backend/com ...