Re: [HACKERS] Re: pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

On 2016-06-15 20:22:24 -0500, Kevin Grittner wrote:
> I'm not clear where you see this as being in any way different with
> STO. Above it seemed that you saw this as an issue related to
> ANALYZE. If there is not early pruning for the table being
> analyzed, nothing is at all different. If there is early pruning
> the rows are not seen and there could be no detoasting. If there
> is a function that lies about IMMUTABLE and reads from a table, it
> either functions as before or throws a STO error on page access
> (long before any detoasting). Am I missing something?

I'm not sure, I might be missing something myself. Given the frequency
of confusion of all senior hackers involved in this discussion...

I previously was thinking of this in the context of ANALYZE, but I now
think it's a bigger problem (and might not really affect ANALYZE
itself).

The simplest version of the scenario I'm concerned about is that a tuple
in a tuple is *not* vacuumed, even though it's elegible to be removed
due to STO. If that tuple has toasted columns, it could be the that the
toast table was independently vacuumed (autovac considers main/toast
tables separately, or the horizon could change between the two heap
scans, or pins could prevent vacuuming of one page, or ...). Toast
accesses via tuptoaster.c currently don't perform
TestForOldSnapshot_impl(), because they use SnapshotToastData, not
SnapshotMVCC.

That seems to means two things:

1) You might get 'missing chunk number ...' errors on access to toasted
columns. Misleading error, but ok.

2) Because the tuple has been pruned from the toast table, it's possible
that the toast oid/va_valueid is reused, because now there's no
conflict with GetNewOidWithIndex() anymore. In that case the
toast_fetch_datum() might return a tuple from another column & type
(all columns in a table share the same toast table), which could lead
to almost arbitrary problems. That's not super likely to happen, but
could have quite severe consequences once it starts.

It seems the easiest way to fix this would be to make
TestForOldSnapshot() "accept" SnapshotToast as well.

Andres