On 04/13/14 14:22, Jan Wieck wrote:
On 04/13/14 08:27, Marko Kreen wrote:
I think you need to do SET_VARSIZE also here.  Alternative is to
move SET_VARSIZE after sort_snapshot().

And it seems the drop-double-txid logic should be added also to
txid_snapshot_recv().  It seems weird to have it behave differently
from txid_snapshot_in().


Thanks,

yes on both issues. Will create another patch.

New patch attached.

New github commit is https://github.com/wieck/postgres/commit/b8fd0d2eb78791e5171e34aecd233fd06218890d


Thanks again,
Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info
diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index a005e67..1023566 100644
*** a/src/backend/utils/adt/txid.c
--- b/src/backend/utils/adt/txid.c
*************** cmp_txid(const void *aa, const void *bb)
*** 131,146 ****
  }
  
  /*
!  * sort a snapshot's txids, so we can use bsearch() later.
   *
   * For consistency of on-disk representation, we always sort even if bsearch
!  * will not be used.
   */
  static void
  sort_snapshot(TxidSnapshot *snap)
  {
        if (snap->nxip > 1)
                qsort(snap->xip, snap->nxip, sizeof(txid), cmp_txid);
  }
  
  /*
--- 131,161 ----
  }
  
  /*
!  * unique sort a snapshot's txids, so we can use bsearch() later.
   *
   * For consistency of on-disk representation, we always sort even if bsearch
!  * will not be used. 
   */
  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);
+               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++;
+               }
+       }
  }
  
  /*
*************** parse_snapshot(const char *str)
*** 294,304 ****
                val = str2txid(str, &endp);
                str = endp;
  
!               /* require the input to be in order */
!               if (val < xmin || val >= xmax || val <= last_val)
                        goto bad_format;
  
!               buf_add_txid(buf, val);
                last_val = val;
  
                if (*str == ',')
--- 309,320 ----
                val = str2txid(str, &endp);
                str = endp;
  
!               /* require the input to be in order and skip duplicates */
!               if (val < xmin || val >= xmax || val < last_val)
                        goto bad_format;
  
!               if (val != last_val)
!                       buf_add_txid(buf, val);
                last_val = val;
  
                if (*str == ',')
*************** txid_current_snapshot(PG_FUNCTION_ARGS)
*** 361,368 ****
  {
        TxidSnapshot *snap;
        uint32          nxip,
!                               i,
!                               size;
        TxidEpoch       state;
        Snapshot        cur;
  
--- 377,383 ----
  {
        TxidSnapshot *snap;
        uint32          nxip,
!                               i;
        TxidEpoch       state;
        Snapshot        cur;
  
*************** txid_current_snapshot(PG_FUNCTION_ARGS)
*** 381,389 ****
  
        /* allocate */
        nxip = cur->xcnt;
!       size = TXID_SNAPSHOT_SIZE(nxip);
!       snap = palloc(size);
!       SET_VARSIZE(snap, size);
  
        /* fill */
        snap->xmin = convert_xid(cur->xmin, &state);
--- 396,402 ----
  
        /* allocate */
        nxip = cur->xcnt;
!       snap = palloc(TXID_SNAPSHOT_SIZE(nxip));
  
        /* fill */
        snap->xmin = convert_xid(cur->xmin, &state);
*************** txid_current_snapshot(PG_FUNCTION_ARGS)
*** 395,400 ****
--- 408,416 ----
        /* we want them guaranteed to be in ascending order */
        sort_snapshot(snap);
  
+       /* we set the size here because the sort may have removed duplicate 
xips */
+       SET_VARSIZE(snap, TXID_SNAPSHOT_SIZE(snap->nxip));
+ 
        PG_RETURN_POINTER(snap);
  }
  
*************** txid_snapshot_recv(PG_FUNCTION_ARGS)
*** 472,489 ****
        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)
                        goto bad_format;
                snap->xip[i] = cur;
                last = cur;
        }
        PG_RETURN_POINTER(snap);
  
  bad_format:
--- 488,514 ----
        snap = palloc(TXID_SNAPSHOT_SIZE(nxip));
        snap->xmin = xmin;
        snap->xmax = xmax;
  
        for (i = 0; i < nxip; i++)
        {
                txid            cur = pq_getmsgint64(buf);
  
!               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 930b86a..7750b7b 100644
*** a/src/test/regress/expected/txid.out
--- b/src/test/regress/expected/txid.out
*************** select '12:18:14,16'::txid_snapshot;
*** 12,17 ****
--- 12,23 ----
   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:"
*************** select '12:16:14,13'::txid_snapshot;
*** 29,38 ****
  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
--- 35,40 ----
diff --git a/src/test/regress/sql/txid.sql b/src/test/regress/sql/txid.sql
index ecae10e..b6650b9 100644
*** a/src/test/regress/sql/txid.sql
--- b/src/test/regress/sql/txid.sql
***************
*** 3,15 ****
  -- i/o
  select '12:13:'::txid_snapshot;
  select '12:18:14,16'::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,
--- 3,15 ----
  -- 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;
  
  create temp table snapshot_test (
        nr      integer,
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to