postgres.git / summary / log / commit / refs

commit    360392fa2a9bb74338b4864790d18eadb13fd8a8
Author:   Amit Kapila <akapila@postgresql.org>
Date:     Mon Nov 27 03:19:55 2023 +0000
First released in: 17.0, 15.6, 16.2

    Avoid unconditionally filling in missing values with NULL in pgoutput.
    
    52e4f0cd4 introduced a bug in pgoutput in which missing values in tuples
    were incorrectly filled in with NULL. The problem was the use of
    CreateTupleDescCopy where CreateTupleDescCopyConstr was required, as the
    former drops the constraints in the tuple description (specifically, the
    default value constraint) on the floor.
    
    The bug could result in incorrectness when a table replicated via
    `REPLICA IDENTITY FULL` underwent a schema change that added a column
    with a default value. The problem is that in such cases updates fill NULL
    values in old tuples for missing columns for default values. Then on the
    subscriber, we failed to find a matching tuple and missed updating the
    required row.
    
    Author: Nikhil Benesch
    Reviewed-by: Hou Zhijie, Amit Kapila
    Backpatch-through: 15
    Discussion: http://postgr.es/m/CAPWqQZTEpZQamYsGMn6ZDRvVywwpVPiKH6OY4KSgA+NmeqFNzA@mail.gmail.com


 src/backend/replication/pgoutput/pgoutput.c |  4 +--
 src/test/subscription/t/100_bugs.pl         | 53 +++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index e8add5ee5d9..f9ed1083df7 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1136,8 +1136,8 @@ init_tuple_slot(PGOutputData *data, Relation relation,
 	 * Create tuple table slots. Create a copy of the TupleDesc as it needs to
 	 * live as long as the cache remains.
 	 */
-	oldtupdesc = CreateTupleDescCopy(RelationGetDescr(relation));
-	newtupdesc = CreateTupleDescCopy(RelationGetDescr(relation));
+	oldtupdesc = CreateTupleDescCopyConstr(RelationGetDescr(relation));
+	newtupdesc = CreateTupleDescCopyConstr(RelationGetDescr(relation));
 
 	entry->old_slot = MakeSingleTupleTableSlot(oldtupdesc, &TTSOpsHeapTuple);
 	entry->new_slot = MakeSingleTupleTableSlot(newtupdesc, &TTSOpsHeapTuple);
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 7e5221afff7..d64be621e81 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -438,4 +438,57 @@ is( $node_subscriber->safe_psql(
 $node_publisher->stop('fast');
 $node_subscriber->stop('fast');
 
+# The bug was that pgoutput was incorrectly replacing missing attributes in
+# tuples with NULL. This could result in incorrect replication with
+# `REPLICA IDENTITY FULL`.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+$node_subscriber->rotate_logfile();
+$node_subscriber->start();
+
+# Set up a table with schema `(a int, b bool)` where the `b` attribute is
+# missing for one row due to the `ALTER TABLE ... ADD COLUMN ... DEFAULT`
+# fast path.
+$node_publisher->safe_psql(
+	'postgres', qq(
+	CREATE TABLE tab_default (a int);
+	ALTER TABLE tab_default REPLICA IDENTITY FULL;
+	INSERT INTO tab_default VALUES (1);
+	ALTER TABLE tab_default ADD COLUMN b bool DEFAULT false NOT NULL;
+	INSERT INTO tab_default VALUES (2, true);
+	CREATE PUBLICATION pub1 FOR TABLE tab_default;
+));
+
+# Replicate to the subscriber.
+$node_subscriber->safe_psql(
+	'postgres', qq(
+	CREATE TABLE tab_default (a int, b bool);
+	CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub1;
+));
+
+$node_subscriber->wait_for_subscription_sync($node_publisher, 'sub1');
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b FROM tab_default");
+is($result, qq(1|f
+2|t), 'check snapshot on subscriber');
+
+# Update all rows in the table and ensure the rows with the missing `b`
+# attribute replicate correctly.
+$node_publisher->safe_psql('postgres',
+	"UPDATE tab_default SET a = a + 1");
+$node_publisher->wait_for_catchup('sub1');
+
+# When the bug is present, the `1|f` row will not be updated to `2|f` because
+# the publisher incorrectly fills in `NULL` for `b` and publishes an update
+# for `1|NULL`, which doesn't exist in the subscriber.
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b FROM tab_default");
+is($result, qq(2|f
+3|t), 'check replicated update on subscriber');
+
+$node_publisher->stop('fast');
+$node_subscriber->stop('fast');
+
 done_testing();


[parent: dc9f8a798307]