On Fri, Feb 08, 2019 at 11:59:05AM +0100, Antonin Houska wrote: > Sorry, I forgot. Patch is below and I'm going to add an entry to the > next CF.
> @@ -615,6 +615,8 @@ SnapBuildInitialSnapshot(SnapBuild *builder) > > TransactionIdAdvance(xid); > } > + /* And of course, adjust snapshot type accordingly. */ > + snap->snapshot_type = SNAPSHOT_MVCC; Wouldn't it be cleaner to have an additional argument to SnapBuildBuildSnapshot() for the snapshot type? It looks confusing to me to overwrite the snapshot type after initializing it once. > @@ -1502,6 +1502,13 @@ ImportSnapshot(const char *idstr) > */ > memset(&snapshot, 0, sizeof(snapshot)); > > + /* > + * Do not rely on the fact that SNAPSHOT_MVCC is zero. (The core code > + * currently does not use this field of imported snapshot, but let's > keep > + * things consistent.) > + */ > + snapshot.snapshot_type = SNAPSHOT_MVCC; Okay for this one, however the comment does not add much value. -- Michael
signature.asc
Description: PGP signature
