On 2019-12-18 18:06:21 +0000, Simon Riggs wrote: > On Wed, 18 Dec 2019 at 17:35, Robert Haas <robertmhaas@gmail.com> wrote: > > > On Wed, Dec 18, 2019 at 10:18 AM Simon Riggs <simon@2ndquadrant.com> > > wrote: > > > This was my first concern when I thought about it, but I realised that > > by taking a snapshot and then calculating xmin normally, this problem would > > go away. > > > > Why? As soon as a transaction aborts... > > > > So this is the same discussion as elsewhere about potentially aborted > transactions... > AFAIK, the worst that happens in that case is that the reading transaction > will end with an ERROR, similar to a serializable error.
I don't think that's all that can happen. E.g. the toast identifier might have been reused, and there might be a different datum in there. Which then means we'll end up calling operators on data that's potentially for a different datatype - it's trivial to cause crashes that way. And, albeit harder, possible to do more than that.
On the patch as proposed this wouldn't be possible because a toast row can't be vacuumed and then reused while holding back xmin, at least as I understand it.
I think there's plenty other problems too, not just toast. There's e.g. some parts of the system that access catalogs using a normal snapshot - which might not actually be consistent, because we have various places where we have to increment the command counter multiple times as part of a larger catalog manipulation.
It seems possible that catalog access would be the thing that makes this difficult. Cache invalidations wouldn't yet have been fired, so that would lead to rather weird errors, and as you say, potential issues from data type changes which would not be acceptable in a facility available to non-superusers.
We could limit that to xacts that don't do DDL, which is a very small % of xacts, but then those xacts are more likely to be ones you'd want to recover or investigate.
So I now withdraw this patch as submitted and won't be resubmitting.