Handle duplicate XIDs in txid_snapshot.

The proc array can contain duplicate XIDs, when a transaction is just being
prepared for two-phase commit. To cope, remove any duplicates in
txid_current_snapshot(). Also ignore duplicates in the input functions, so
that if e.g. you have an old pg_dump file that already contains duplicates,
it will be accepted.

Report and fix by Jan Wieck. Backpatch to all supported versions.

diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index 6a8a8803836aa922d6b9a6528f42d6cea16837e3..843302682b6eb379be78314bbf804574f0843fd2 100644 (file)
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -130,7 +130,8 @@ cmp_txid(const void *aa, const void *bb)
 }
 
 /*
- * sort a snapshot's txids, so we can use bsearch() later.
+ * Sort a snapshot's txids, so we can use bsearch() later.  Also remove
+ * any duplicates.
  *
  * For consistency of on-disk representation, we always sort even if bsearch
  * will not be used.
@@ -138,8 +139,25 @@ cmp_txid(const void *aa, const void *bb)
 static void
 sort_snapshot(TxidSnapshot *snap)
 {
+   txid    last = 0;
+   int     nxip, idx1, idx2;
+
    if (snap->nxip > 1)
+   {
        qsort(snap->xip, snap->nxip, sizeof(txid), cmp_txid);
+
+       /* remove duplicates */
+       nxip = snap->nxip;
+       idx1 = idx2 = 0;
+       while (idx1 < nxip)
+       {
+           if (snap->xip[idx1] != last)
+               last = snap->xip[idx2++] = snap->xip[idx1];
+           else
+               snap->nxip--;
+           idx1++;
+       }
+   }
 }
 
 /*
@@ -294,10 +312,12 @@ parse_snapshot(const char *str)
        str = endp;
 
        /* require the input to be in order */
-       if (val < xmin || val >= xmax || val <= last_val)
+       if (val < xmin || val >= xmax || val < last_val)
            goto bad_format;
 
-       buf_add_txid(buf, val);
+       /* skip duplicates */
+       if (val != last_val)
+           buf_add_txid(buf, val);
        last_val = val;
 
        if (*str == ',')
@@ -359,8 +379,7 @@ txid_current_snapshot(PG_FUNCTION_ARGS)
 {
    TxidSnapshot *snap;
    uint32      nxip,
-               i,
-               size;
+               i;
    TxidEpoch   state;
    Snapshot    cur;
 
@@ -372,9 +391,7 @@ txid_current_snapshot(PG_FUNCTION_ARGS)
 
    /* allocate */
    nxip = cur->xcnt;
-   size = TXID_SNAPSHOT_SIZE(nxip);
-   snap = palloc(size);
-   SET_VARSIZE(snap, size);
+   snap = palloc(TXID_SNAPSHOT_SIZE(nxip));
 
    /* fill */
    snap->xmin = convert_xid(cur->xmin, &state);
@@ -383,9 +400,18 @@ txid_current_snapshot(PG_FUNCTION_ARGS)
    for (i = 0; i < nxip; i++)
        snap->xip[i] = convert_xid(cur->xip[i], &state);
 
-   /* we want them guaranteed to be in ascending order */
+   /*
+    * We want them guaranteed to be in ascending order.  This also removes
+    * any duplicate xids.  Normally, an XID can only be assigned to one
+    * backend, but when preparing a transaction for two-phase commit, there
+    * is a transient state when both the original backend and the dummy
+    * PGPROC entry reserved for the prepared transaction hold the same XID.
+    */
    sort_snapshot(snap);
 
+   /* set size after sorting, because it may have removed duplicate xips */
+   SET_VARSIZE(snap, TXID_SNAPSHOT_SIZE(snap->nxip));
+
    PG_RETURN_POINTER(snap);
 }
 
@@ -463,18 +489,27 @@ txid_snapshot_recv(PG_FUNCTION_ARGS)
    snap = palloc(TXID_SNAPSHOT_SIZE(nxip));
    snap->xmin = xmin;
    snap->xmax = xmax;
-   snap->nxip = nxip;
-   SET_VARSIZE(snap, TXID_SNAPSHOT_SIZE(nxip));
 
    for (i = 0; i < nxip; i++)
    {
        txid        cur = pq_getmsgint64(buf);
 
-       if (cur <= last || cur < xmin || cur >= xmax)
+       if (cur < last || cur < xmin || cur >= xmax)
            goto bad_format;
+
+       /* skip duplicate xips */
+       if (cur == last)
+       {
+           i--;
+           nxip--;
+           continue;
+       }
+
        snap->xip[i] = cur;
        last = cur;
    }
+   snap->nxip = nxip;
+   SET_VARSIZE(snap, TXID_SNAPSHOT_SIZE(nxip));
    PG_RETURN_POINTER(snap);
 
 bad_format:

diff --git a/src/test/regress/expected/txid.out b/src/test/regress/expected/txid.out
index 930b86a65628482cc4e841f89ffbab6055cfe720..7750b7b98f937b2c81a80dc0dba91e34a34c33fb 100644 (file)
--- a/src/test/regress/expected/txid.out
+++ b/src/test/regress/expected/txid.out
@@ -12,6 +12,12 @@ select '12:18:14,16'::txid_snapshot;
  12:18:14,16
 (1 row)
 
+select '12:16:14,14'::txid_snapshot;
+ txid_snapshot 
+---------------
+ 12:16:14
+(1 row)
+
 -- errors
 select '31:12:'::txid_snapshot;
 ERROR:  invalid input for txid_snapshot: "31:12:"
@@ -29,10 +35,6 @@ select '12:16:14,13'::txid_snapshot;
 ERROR:  invalid input for txid_snapshot: "12:16:14,13"
 LINE 1: select '12:16:14,13'::txid_snapshot;
                ^
-select '12:16:14,14'::txid_snapshot;
-ERROR:  invalid input for txid_snapshot: "12:16:14,14"
-LINE 1: select '12:16:14,14'::txid_snapshot;
-               ^
 create temp table snapshot_test (
    nr  integer,
    snap    txid_snapshot

diff --git a/src/test/regress/sql/txid.sql b/src/test/regress/sql/txid.sql
index ecae10e024d3961cab062698e421213293ea856b..b6650b922e6d4b0df87fde088be491600b386e2f 100644 (file)
--- a/src/test/regress/sql/txid.sql
+++ b/src/test/regress/sql/txid.sql
@@ -3,13 +3,13 @@
 -- i/o
 select '12:13:'::txid_snapshot;
 select '12:18:14,16'::txid_snapshot;
+select '12:16:14,14'::txid_snapshot;
 
 -- errors
 select '31:12:'::txid_snapshot;
 select '0:1:'::txid_snapshot;
 select '12:13:0'::txid_snapshot;
 select '12:16:14,13'::txid_snapshot;
-select '12:16:14,14'::txid_snapshot;
 
 create temp table snapshot_test (
    nr  integer,