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
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 ... |