Hello, Antonin!
> The changes present in WAL decoded prior the snapshot creation are not
> replayed - these changes are visible to the snapshot. (This is not really
> specific to the 0006 part.)
OK, just want to be sure it still works the same way if we build multiple snapshots for the same slot that way.
> The current API does not seem to support changing snapshot of an in-progress
> scan and I don't want to change that. Plus note that the current
> implementation of CLUSTER also uses SnapshotAny and then checks the visibility
> separately. Finally, SnapshotAny is not really an expensive visibility check,
> if it can be considered a visibility check at all.
But we will require a real check for each tuple. Including dead one, multiple versions of the same HOT, etc.
> I've added it only for xmin. xid is valid because REPACK is executed in a
> transaction. That reminds me that PROC_IN_VACUUM should be present in
> MyProc->statusFlags. Fixed.
Yes, xid is required for repack. I think it is better to introduce a new flag instead of PROC_IN_VACCUUM.
> > > PushActiveSnapshot(GetTransactionSnapshot());
> > GetLatestSnapshot() feels better here.
> What will then happen to code that uses GetActiveSnapshot() ?
O, I mean PushActiveSnapshot(GetLatestSnapshot())
> > Also, to correctly build a unique index - some tech from [0] is required (building a unique index with multiple snapshots is a little bit tricky).
> ok, I'll check your patch.
I realized building a unique index is still done with a single snapshot, so it should be OK for that case. But still check the patch :)
> I proposed the Assert above, but still thinking about it.
Hm... Do we really need these asserts if PROC_IN_VACUUM is set? I was proposing a way it is used for index building (to ensure nothing is propagated into xmin).
Best regards,
Mikhail.
