On Sun, Mar 04, 2012 at 01:02:57PM +0000, Simon Riggs wrote:
> More detailed thoughts show that the test in heap_beginscan_internal()
> is not right enough, i.e. wrong.
>
> We need a specific XidInMVCCSnapshot test on the relvalidxid, so it
> needs to be a specific xid, not an xmin because otherwise we can get
> concurrent transactions failing, not just older transactions.
Good point; I agree. indcheckxmin's level of pessimism isn't appropriate for
this new check.
> If we're going freeze tuples on load this needs to be watertight, so
> some minor rework needed.
>
> Of course, if we only have a valid xid on the class we might get new
> columns added when we do repeated SELECT * statements using the same
> snapshot while concurrent DDL occurs. That is impractical, so if we
> define this as applying to rows it can work; if we want it to apply to
> everything its getting more difficult.
Excess columns seem less grave to me than excess or missing rows. I'm having
difficulty thinking up an explanation for that opinion.