Don't propagate a null subtransaction snapshot up to parent transaction.

This oversight could cause logical decoding to fail to decode an outer
transaction containing changes, if a subtransaction had an XID but no
actual changes.  Per bug #14279 from Marko Tiikkaja.  Patch by Marko
based on analysis by Andrew Gierth.

Discussion: <20160804191757.1430.39011@wrigleys.postgresql.org>

diff --git a/contrib/test_decoding/expected/xact.out b/contrib/test_decoding/expected/xact.out
index 507b701c3ab593880a0a26fcc53c2f0e439e8d55..ec4745005d760b4818d0ec9843df28182fc03c97 100644 (file)
--- a/contrib/test_decoding/expected/xact.out
+++ b/contrib/test_decoding/expected/xact.out
@@ -6,9 +6,9 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_d
  init
 (1 row)
 
--- bug #13844, xids in non-decoded records need to be inspected
 CREATE TABLE xact_test(data text);
 INSERT INTO xact_test VALUES ('before-test');
+-- bug #13844, xids in non-decoded records need to be inspected
 BEGIN;
 -- perform operation in xact that creates and logs xid, but isn't decoded
 SELECT * FROM xact_test FOR UPDATE;
@@ -33,6 +33,28 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
  COMMIT
 (6 rows)
 
+-- bug #14279, do not propagate null snapshot from subtransaction
+BEGIN;
+-- first insert
+INSERT INTO xact_test VALUES ('main-txn');
+SAVEPOINT foo;
+-- now perform operation in subxact that creates and logs xid, but isn't decoded
+SELECT 1 FROM xact_test FOR UPDATE LIMIT 1;
+ ?column? 
+----------
+        1
+(1 row)
+
+COMMIT;
+-- and now show those changes
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+                         data                          
+-------------------------------------------------------
+ BEGIN
+ table public.xact_test: INSERT: data[text]:'main-txn'
+ COMMIT
+(3 rows)
+
 DROP TABLE xact_test;
 SELECT pg_drop_replication_slot('regression_slot');
  pg_drop_replication_slot 

diff --git a/contrib/test_decoding/sql/xact.sql b/contrib/test_decoding/sql/xact.sql
index 9ce238f62dfc0c6415ca6edb2b6909bec464122f..aa555911e868e8654c85b92dc6b9ba945c7b27a9 100644 (file)
--- a/contrib/test_decoding/sql/xact.sql
+++ b/contrib/test_decoding/sql/xact.sql
@@ -3,10 +3,10 @@ SET synchronous_commit = on;
 
 SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
 
--- bug #13844, xids in non-decoded records need to be inspected
 CREATE TABLE xact_test(data text);
 INSERT INTO xact_test VALUES ('before-test');
 
+-- bug #13844, xids in non-decoded records need to be inspected
 BEGIN;
 -- perform operation in xact that creates and logs xid, but isn't decoded
 SELECT * FROM xact_test FOR UPDATE;
@@ -17,6 +17,17 @@ COMMIT;
 -- and now show those changes
 SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
 
+-- bug #14279, do not propagate null snapshot from subtransaction
+BEGIN;
+-- first insert
+INSERT INTO xact_test VALUES ('main-txn');
+SAVEPOINT foo;
+-- now perform operation in subxact that creates and logs xid, but isn't decoded
+SELECT 1 FROM xact_test FOR UPDATE LIMIT 1;
+COMMIT;
+-- and now show those changes
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+
 DROP TABLE xact_test;
 
 SELECT pg_drop_replication_slot('regression_slot');

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 77375d91b28c69b8d352a373983e73722b7028aa..9594b1c671b4e67b375c69c9749dc0e9698af302 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -807,13 +807,14 @@ ReorderBufferCommitChild(ReorderBuffer *rb, TransactionId xid,
                elog(ERROR, "subxact logged without previous toplevel record");
 
        /*
-        * Pass the our base snapshot to the parent transaction if it doesn't have
+        * Pass our base snapshot to the parent transaction if it doesn't have
         * one, or ours is older. That can happen if there are no changes in the
         * toplevel transaction but in one of the child transactions. This allows
-        * the parent to simply use it's base snapshot initially.
+        * the parent to simply use its base snapshot initially.
         */
-       if (txn->base_snapshot == NULL ||
-               txn->base_snapshot_lsn > subtxn->base_snapshot_lsn)
+       if (subtxn->base_snapshot != NULL &&
+               (txn->base_snapshot == NULL ||
+                txn->base_snapshot_lsn > subtxn->base_snapshot_lsn))
        {
                txn->base_snapshot = subtxn->base_snapshot;
                txn->base_snapshot_lsn = subtxn->base_snapshot_lsn;