Skip site navigation (1) Skip section navigation (2)

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: (view raw, whole thread or download thread mbox)
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


pgsql-patches by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group