logical decoding: old/newtuple in spooled UPDATE changes was switched around.

Somehow I managed to flip the order of restoring old & new tuples when
de-spooling a change in a large transaction from disk. This happens to
only take effect when a change is spooled to disk which has old/new
versions of the tuple. That only is the case for UPDATEs where he
primary key changed or where replica identity is changed to FULL.

The tests didn't catch this because either spooled updates, or updates
that changed primary keys, were tested; not both at the same time.

Found while adding tests for the following commit.

Backpatch: 9.4, where logical decoding was added

diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out
index 8e8eb5965268a99590f85e15290546cdaf3f2e40..a3ed9a5cbf98d3e0fddf1e4c682b846c9069ad98 100644 (file)
--- a/contrib/test_decoding/expected/ddl.out
+++ b/contrib/test_decoding/expected/ddl.out
@@ -196,6 +196,21 @@ ORDER BY 1,2;
  20467 | table public.tr_etoomuch: DELETE: id[integer]:1 | table public.tr_etoomuch: UPDATE: id[integer]:9999 data[integer]:-9999
 (3 rows)
 
+-- check updates of primary keys work correctly
+BEGIN;
+CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
+UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
+DELETE FROM spoolme;
+DROP TABLE spoolme;
+COMMIT;
+SELECT data
+FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1')
+WHERE data ~ 'UPDATE';
+                                                    data                                                     
+-------------------------------------------------------------------------------------------------------------
+ table public.tr_etoomuch: UPDATE: old-key: id[integer]:5000 new-tuple: id[integer]:-5000 data[integer]:5000
+(1 row)
+
 /*
  * check whether we decode subtransactions correctly in relation with each
  * other

diff --git a/contrib/test_decoding/sql/ddl.sql b/contrib/test_decoding/sql/ddl.sql
index 7357902e5905a3c44fe898a477c87179cbca7461..8e3dae629317b746cf0cdadbba8795ef2974f0f2 100644 (file)
--- a/contrib/test_decoding/sql/ddl.sql
+++ b/contrib/test_decoding/sql/ddl.sql
@@ -114,6 +114,18 @@ FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids',
 GROUP BY substring(data, 1, 24)
 ORDER BY 1,2;
 
+-- check updates of primary keys work correctly
+BEGIN;
+CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
+UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
+DELETE FROM spoolme;
+DROP TABLE spoolme;
+COMMIT;
+
+SELECT data
+FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1')
+WHERE data ~ 'UPDATE';
+
 /*
  * check whether we decode subtransactions correctly in relation with each
  * other

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5b434ac825f9f7dba09f3cc1effaeadfc49894b9..22b83ab6ad03f920239493ddbe95a437fb7ac86c 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2266,29 +2266,29 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
        case REORDER_BUFFER_CHANGE_UPDATE:
            /* fall through */
        case REORDER_BUFFER_CHANGE_DELETE:
-           if (change->data.tp.newtuple)
+           if (change->data.tp.oldtuple)
            {
                Size        len = offsetof(ReorderBufferTupleBuf, data)
                +((ReorderBufferTupleBuf *) data)->tuple.t_len
                - offsetof(HeapTupleHeaderData, t_bits);
 
-               change->data.tp.newtuple = ReorderBufferGetTupleBuf(rb);
-               memcpy(change->data.tp.newtuple, data, len);
-               change->data.tp.newtuple->tuple.t_data =
-                   &change->data.tp.newtuple->header;
+               change->data.tp.oldtuple = ReorderBufferGetTupleBuf(rb);
+               memcpy(change->data.tp.oldtuple, data, len);
+               change->data.tp.oldtuple->tuple.t_data =
+                   &change->data.tp.oldtuple->header;
                data += len;
            }
 
-           if (change->data.tp.oldtuple)
+           if (change->data.tp.newtuple)
            {
                Size        len = offsetof(ReorderBufferTupleBuf, data)
                +((ReorderBufferTupleBuf *) data)->tuple.t_len
                - offsetof(HeapTupleHeaderData, t_bits);
 
-               change->data.tp.oldtuple = ReorderBufferGetTupleBuf(rb);
-               memcpy(change->data.tp.oldtuple, data, len);
-               change->data.tp.oldtuple->tuple.t_data =
-                   &change->data.tp.oldtuple->header;
+               change->data.tp.newtuple = ReorderBufferGetTupleBuf(rb);
+               memcpy(change->data.tp.newtuple, data, len);
+               change->data.tp.newtuple->tuple.t_data =
+                   &change->data.tp.newtuple->header;
                data += len;
            }
            break;