Nitpick about assumptions in indexam and tableam

Hackers,

The return value of RegisterSnapshot is being ignored in a
few places in indexam.c and tableam.c, suggesting an
intimate knowledge of the inner workings of the snapshot
manager from these two files. I don't think that is particularly
wise, and I don't see a performance justification for the way
it is presently coded. There are no live bugs caused by this
that I can see, but I would still like it cleaned up.

Inside index_beginscan_parallel:

snapshot = RestoreSnapshot(pscan->ps_snapshot_data);
RegisterSnapshot(snapshot);
scan = index_beginscan_internal(indexrel, nkeys, norderbys, snapshot,
pscan, true);

It happens to be true in the current implementation of the
snapshot manager that restored snapshots will have their
'copied' field set to true, and that the RegisterSnapshot
function will in that case return the same snapshot that
it was handed, so the snapshot handed to index_beginscan_internal
turns out to be the right one. But if RegisterSnapshot were
changed to return a different copy of the snapshot, this code
would break.

There is a similar level of knowledge in table_beginscan_parallel,
which for brevity I won't quote here.

The code in table_scan_update_snapshot appears even more
brittle to me. The only function in the core code base that
calls table_scan_update_snapshot is ExecBitmapHeapInitializeWorker,
and it does so right after restoring the snapshot that it hands
to table_scan_update_snapshot, so the fact that
table_scan_update_snapshot then ignores the return value
of RegisterSnapshot on that snapshot happens to be ok. If
some other code were changed to call this function, it is not
clear that it would work out so well.

I propose that attached patch.

mark