"Marko Kreen" <markokr at gmail.com> writes:
> For reference I attached v4 of patch, with your changes except
> sorting related applied.

A few random comments:

+++ b/contrib/txid/Makefile
+
+
+test: install
+	$(MAKE) installcheck || { less regression.diffs; exit 1; }
+
+ack:
+	cp results/* expected/

Please drop these random, unportable makefile targets.

+++ b/contrib/txid/README.txid

Needs some spellchecking...

+++ b/contrib/txid/txid.c

+convert_xid(TransactionId xid, const TxidEpoch *state)
+{
+	uint64 epoch;
+
+	/* avoid issues with the the special meaning of 0 */
+	if (xid == InvalidTransactionId)
+		return MAX_TXID;
+
+	/* return special xid's as-is */
+	if (xid < FirstNormalTransactionId)
+		return xid;

I'm having quite a bit of a problem with the above.  Why is
InvalidTransactionId mapped to MAX_TXID, which presumably is part of the
normal XID rotation and hence only larger than half of the universe,
when the other special XIDs map as themselves?

+/*
+ * helper functions to use StringInfo for TxidSnapshot creation.
+ */

I'm kind of wondering where this idea came from at all --- it seems
like the very worst sort of impedance mismatch.  StringInfo really
is not intended for non-textual data...

			regards, tom lane