Incorrect visibility test function assigned to snapshot

In the header comment, SnapBuildInitialSnapshot() claims to set
snapshot->satisfies to the HeapTupleSatisfiesMVCC test function, and indeed it
converts the "xid" array to match its semantics (i.e. the xid items eventually
represent running transactions as opposed to the committed ones). However the
test function remains HeapTupleSatisfiesHistoricMVCC as set by
SnapBuildBuildSnapshot().

I suppose this is a bug: HeapTupleSatisfiesHistoricMVCC expects the committed
transactions in the snapshot->subxip array, however the snapshot returned by
SnapBuildInitialSnapshot() leaves this array empty. And even if the function
used snapshot->xip, it'd find the running transactions there instead of those
committed.

This is what I propose as a fix:

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
new file mode 100644
index 4123cde..53e8b95
*** a/src/backend/replication/logical/snapbuild.c
--- b/src/backend/replication/logical/snapbuild.c
*************** SnapBuildInitialSnapshot(SnapBuild *buil
*** 619,624 ****
--- 619,625 ----

        snap->xcnt = newxcnt;
        snap->xip = newxip;
+       snap->satisfies = HeapTupleSatisfiesMVCC;

        return snap;
  }

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com