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 a48d42c5a142217a26c7ecdfe3c1a641068e5698..f0498aa302ce4c0be611491aa878c97a4e07658c 100644 (file)
--- a/contrib/test_decoding/expected/ddl.out
+++ b/contrib/test_decoding/expected/ddl.out
@@ -240,6 +240,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 that a large, spooled, upsert works
 INSERT INTO tr_etoomuch (id, data)
 SELECT g.i, -g.i FROM generate_series(8000, 12000) g(i)

diff --git a/contrib/test_decoding/sql/ddl.sql b/contrib/test_decoding/sql/ddl.sql
index e311c5966e00701dcb1f3735ec328d7eefc678e2..ad928ad572688e06a17ed352f4e6534078b69950 100644 (file)
--- a/contrib/test_decoding/sql/ddl.sql
+++ b/contrib/test_decoding/sql/ddl.sql
@@ -123,6 +123,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 that a large, spooled, upsert works
 INSERT INTO tr_etoomuch (id, data)
 SELECT g.i, -g.i FROM generate_series(8000, 12000) g(i)

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index b3276c74c7c1303e4e3fd24aaa31b5880562a367..2f4489d1b3ec44164a270293b167e90d89c56315 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2335,27 +2335,27 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
        case REORDER_BUFFER_CHANGE_UPDATE:
        case REORDER_BUFFER_CHANGE_DELETE:
        case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT:
-           if (change->data.tp.newtuple)
+           if (change->data.tp.oldtuple)
            {
                Size        len = offsetof(ReorderBufferTupleBuf, t_data) +
                ((ReorderBufferTupleBuf *) data)->tuple.t_len;
 
-               change->data.tp.newtuple = ReorderBufferGetTupleBuf(rb);
-               memcpy(change->data.tp.newtuple, data, len);
-               change->data.tp.newtuple->tuple.t_data =
-                   &change->data.tp.newtuple->t_data.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->t_data.header;
                data += len;
            }
 
-           if (change->data.tp.oldtuple)
+           if (change->data.tp.newtuple)
            {
                Size        len = offsetof(ReorderBufferTupleBuf, t_data) +
                ((ReorderBufferTupleBuf *) data)->tuple.t_len;
 
-               change->data.tp.oldtuple = ReorderBufferGetTupleBuf(rb);
-               memcpy(change->data.tp.oldtuple, data, len);
-               change->data.tp.oldtuple->tuple.t_data =
-                   &change->data.tp.oldtuple->t_data.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->t_data.header;
                data += len;
            }
            break;