Potential vacuum bug?

While chasing the VACUUM problem reported by Stephen Birch, I noticed
something that looks like a potential trouble spot. Vacuum's initial
scan routine, vc_scanheap, runs through the table to mark tuples as
known committed or known dead, if possible (consulting the transaction
log for tuples not yet so marked). It does the right things as far as
marking committed/dead if it sees a tuple marked HEAP_MOVED_OFF or
HEAP_MOVED_IN, which could only be there if a prior VACUUM failed
partway through. But it doesn't *clear* those bits. Seems to me that
that will screw up the subsequent vc_rpfheap procedure --- in
particular, leaving a HEAP_MOVED_IN flag set will cause vc_rpfheap to
complain (correctly!) about 'HEAP_MOVED_IN not expected', whereas
leaving HEAP_MOVED_OFF set will confuse vc_rpfheap because it will
think it moved the tuple itself.

In short, if we really want to recover from a failed VACUUM then we'd
better clear those bits during vc_scanheap. I am thinking that the
code starting at about line 720 ought to look like

if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
{
if (tuple.t_data->t_infomask & HEAP_XMIN_INVALID)
tupgone = true;
else if (tuple.t_data->t_infomask & HEAP_MOVED_OFF)
{
// mark tuple commited or invalid as appropriate,
// same as before
add >>> tuple.t_data->t_infomask &= ~HEAP_MOVED_OFF;
}
else if (tuple.t_data->t_infomask & HEAP_MOVED_IN)
{
// mark tuple commited or invalid as appropriate,
// same as before
add >>> tuple.t_data->t_infomask &= ~HEAP_MOVED_IN;
}
else
{
// other cases same as before
}
}

add >>> if (tuple.t_data->t_infomask & (HEAP_MOVED_OFF | HEAP_MOVED_IN))
add >>> {
add >>> elog(NOTICE, "Clearing unexpected HEAP_MOVED flag");
add >>> tuple.t_data->t_infomask &= ~(HEAP_MOVED_OFF | HEAP_MOVED_IN);
add >>> pgchanged = true;
add >>> }

Comments anyone?

regards, tom lane