On Tue, Nov 15, 2022 at 8:08 AM Andres Freund <and...@anarazel.de> wrote: > > On 2022-11-14 17:25:31 -0800, Andres Freund wrote: > > Hm, also, shouldn't the patch adding CRS_USE_SNAPSHOT have copied more of > > SnapBuildExportSnapshot()? Why aren't the error checks for > > SnapBuildExportSnapshot() needed? Why don't we need to set XactReadOnly? > > Which > > transactions are we even in when we import the snapshot (cf. > > SnapBuildExportSnapshot() doing a StartTransactionCommand()). > > Most of the checks for that are in CreateReplicationSlot() - but not al, > e.g. XactReadOnly isn't set, >
Yeah, I think we can add the check for XactReadOnly along with other checks in CreateReplicationSlot(). > nor do we enforce in an obvious place that we > don't already hold a snapshot. > We have a check for (FirstXactSnapshot == NULL) in RestoreTransactionSnapshot->SetTransactionSnapshot. Won't that be sufficient? -- With Regards, Amit Kapila.
