PS_NITPICKS_20240705_GENCOLS_V140002.txt

text/plain

Filename: PS_NITPICKS_20240705_GENCOLS_V140002.txt
Type: text/plain
Part: 0
Message: Re: Pgoutput not capturing the generated columns
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 38f3621..c264353 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -704,30 +704,30 @@ make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist)
 	TupleDesc	desc;
 
 	desc = RelationGetDescr(rel->localrel);
-	localgenlist = palloc0((rel->remoterel.natts * sizeof(bool)));
+	localgenlist = palloc0(rel->remoterel.natts * sizeof(bool));
 
 	/*
 	 * This loop checks for generated columns on subscription table.
 	 */
 	for (int i = 0; i < desc->natts; i++)
 	{
-		int			attnum;
+		int			remote_attnum;
 		Form_pg_attribute attr = TupleDescAttr(desc, i);
 
 		if (!attr->attgenerated)
 			continue;
 
-		attnum = logicalrep_rel_att_by_name(&rel->remoterel,
+		remote_attnum = logicalrep_rel_att_by_name(&rel->remoterel,
 											NameStr(attr->attname));
 
-		if (attnum >= 0)
+		if (remote_attnum >= 0)
 		{
 			/*
 			 * Check if the subscription table generated column has same
 			 * name as a non-generated column in the corresponding
 			 * publication table.
 			 */
-			if (!remotegenlist[attnum])
+			if (!remotegenlist[remote_attnum])
 				ereport(ERROR,
 						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 						 errmsg("logical replication target relation \"%s.%s\" has a generated column \"%s\" "
@@ -739,7 +739,7 @@ make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist)
 			 * the subscription table. Later, we use this information to
 			 * skip adding this column to the column list for COPY.
 			 */
-			localgenlist[attnum] = true;
+			localgenlist[remote_attnum] = true;
 		}
 	}
 
@@ -1205,12 +1205,12 @@ copy_table(Relation rel)
 	/* Start copy on the publisher. */
 	initStringInfo(&cmd);
 
-	/* check if remote column list has generated columns */
+	/* Check if remote column list has generated columns */
 	if(MySubscription->includegencols)
 	{
 		for (int i = 0; i < relmapentry->remoterel.natts; i++)
 		{
-			if(remotegenlist[i])
+			if (remotegenlist[i])
 			{
 				remote_has_gencol = true;
 				break;
@@ -1219,8 +1219,8 @@ copy_table(Relation rel)
 	}
 
 	/*
-	 * Regular table with no row filter and 'include_generated_columns'
-	 * specified as 'false' during creation of subscription.
+	 * Regular table with no row filter and copy of generated columns is
+	 * not necessary.
 	 */
 	if (lrel.relkind == RELKIND_RELATION && qual == NIL && !remote_has_gencol)
 	{
@@ -1258,8 +1258,9 @@ copy_table(Relation rel)
 		 * SELECT query with OR'ed row filters for COPY.
 		 *
 		 * We also need to use this same COPY (SELECT ...) syntax when
-		 * 'include_generated_columns' is specified as true, because copy
-		 * of generated columns is not supported by the normal COPY.
+		 * 'include_generated_columns' is specified as true and the remote
+		 * table has generated columns, because copy of generated columns is
+		 * not supported by the normal COPY.
 		 */
 		int i = 0;
 
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index 68052e1..62462c0 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -176,18 +176,24 @@ ok( $node_publisher->poll_query_until(
 $node_publisher->safe_psql('postgres', "DROP TABLE tab_rep");
 $node_subscriber->safe_psql('postgres', "DROP TABLE tab_rep");
 
-# When a subscription table have a column missing as specified on publication table
+#
+# TEST CASE:
+#
+# When a subscription table has a column missing that was specified on
+# the publication table.
+#
+
 # setup structure with existing data on publisher
 $node_publisher->safe_psql('postgres', "CREATE TABLE tab_rep (a int, b int)");
 $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab_rep VALUES (1, 1), (2, 2), (3, 3)");
 
-# add table on subscriber
+# add table on subscriber; note column 'b' is missing
 $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_rep (a int)");
 
 my $offset = -s $node_subscriber->logfile;
 
-# recreate the subscription again
+# create the subscription
 $node_subscriber->safe_psql('postgres',
 	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
 );
@@ -201,15 +207,23 @@ $node_subscriber->wait_for_log(
 $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
 $node_subscriber->safe_psql('postgres', "DROP TABLE tab_rep");
 
-# When a subscription table have a generated column corresponding to non-generated column on publication table
-# create table on subscriber side with generated column
+#
+# TEST CASE:
+#
+# When a subscription table has a generated column corresponding to a
+# non-generated column on publication table
+#
+
+# create table on subscriber side with generated column 'b'
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab_rep (a int, b int GENERATED ALWAYS AS (a * 2) STORED)");
+
+# create the subscription
 $node_subscriber->safe_psql('postgres',
 	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
 );
 
-# check for missing column error
+# check for generated column mismatch error
 $node_subscriber->wait_for_log(
 	qr/ERROR: ( [A-Z0-9]+:)? logical replication target relation "public.tab_rep" has a generated column "b" but corresponding column on source relation is not a generated column/,
 	$offset);
diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl
index 2628ad3..9569f57 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -35,33 +35,37 @@ $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab2 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)");
 $node_subscriber->safe_psql('postgres', "CREATE TABLE tab2 (a int, b int)");
 
-# publisher-side tab3 has generated col 'b' but subscriber-side tab3 has DIFFERENT COMPUTATION generated col 'b'.
+# tab3:
+# publisher-side tab3 has generated col 'b' but
+# subscriber-side tab3 has DIFFERENT COMPUTATION generated col 'b'.
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 10) STORED)");
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 20) STORED)");
 
-# publisher-side tab4 has generated cols 'b' and 'c' but subscriber-side tab4 has non-generated col 'b', and generated-col 'c'
-# where columns on the subscriber are in a different order
+# tab4:
+# publisher-side tab4 has generated cols 'b' and 'c' but
+# subscriber-side tab4 has non-generated col 'b', and generated-col 'c'
+# where columns on publisher/subscriber are in a different order
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab4 (a int, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (a * 2) STORED)"
 );
-
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab4 (c int GENERATED ALWAYS AS (a * 22) STORED, a int, b int)"
 );
 
-# publisher-side tab5 has non-generated col 'b' but subscriber-side tab5 has generated col 'b'
+# tab5:
+# publisher-side tab5 has non-generated col 'b' but
+# subscriber-side tab5 has generated col 'b'
 $node_publisher->safe_psql('postgres', "CREATE TABLE tab5 (a int, b int)");
-
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab5 (a int, b int GENERATED ALWAYS AS (a * 22) STORED)");
 
-# test for alter subscription ... refresh publication
+# tab6:
+# tables for testing ALTER SUBSCRIPTIO ... REFRESH PUBLICATION
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab6 (a int, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (a * 2) STORED)"
 );
-
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab6 (a int, b int, c int GENERATED ALWAYS AS (a * 22) STORED)"
 );
@@ -129,6 +133,10 @@ is( $result, qq(1|2|22
 2|4|44
 3|6|66), 'generated column initial sync');
 
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b, c FROM tab6 ORDER BY a");
+is( $result, qq(), 'unsubscribed table initial data');
+
 # data to replicate
 
 $node_publisher->safe_psql('postgres', "INSERT INTO tab1 VALUES (4), (5)");
@@ -178,9 +186,11 @@ is( $result, qq(1|21
 	'confirm generated columns are NOT replicated when the subscriber-side column is also generated'
 );
 
+#
 # TEST tab4: the publisher-side cols 'b' and 'c' are generated and subscriber-side
 # col 'b' is not generated and col 'c' is generated. So confirmed that the different
-# order of columns on subsriber-side replicate data to correct columns.
+# order of columns on subscriber-side replicate data to correct columns.
+#
 $node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (4), (5)");
 $node_publisher->wait_for_catchup('sub4');
 $result =
@@ -192,12 +202,26 @@ is( $result, qq(1|2|22
 4|8|88
 5|10|110), 'replicate generated columns with different order on subscriber');
 
-# TEST for ALTER SUBSCRIPTION ... REFRESH PUBLICATION
-# Add a new table to publication
+#
+# TEST tab5: publisher-side col 'b' is not-generated and subscriber-side col 'b'
+# is generated, so confirmed that col 'b' IS NOT replicated and it will throw an error.
+# The subscription sub5 is created here, instead of earlier with the other subscriptions,
+# because sub5 will cause the tablesync worker to restart repetitively.
+#
+my $offset = -s $node_subscriber->logfile;
+$node_subscriber->safe_psql('postgres',
+	"CREATE SUBSCRIPTION sub5 CONNECTION '$publisher_connstr' PUBLICATION pub5 WITH (include_generated_columns = true)"
+);
+$node_subscriber->wait_for_log(
+	qr/ERROR: ( [A-Z0-9]:)? logical replication target relation "public.tab5" has a generated column "b" but corresponding column on source relation is not a generated column/,
+	$offset);
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub5");
+
+#
+# TEST tab6: After ALTER SUBSCRIPTION ... REFRESH PUBLICATION
+#
 $node_publisher->safe_psql('postgres',
 	"ALTER PUBLICATION pub4 ADD TABLE tab6");
-
-# Refresh publication after table is added to publication
 $node_subscriber->safe_psql('postgres',
 	"ALTER SUBSCRIPTION sub4 REFRESH PUBLICATION");
 $node_publisher->wait_for_catchup('sub4');
@@ -207,7 +231,10 @@ is( $result, qq(1|2|22
 2|4|44
 3|6|66), 'add new table to existing publication');
 
-# TEST: drop generated column on subscriber side
+#
+# TEST tab6: Drop the generated column's expression on subscriber side.
+# This changes the generated column into a non-generated column.
+#
 $node_subscriber->safe_psql('postgres',
 	"ALTER TABLE tab6 ALTER COLUMN c DROP EXPRESSION");
 $node_publisher->safe_psql('postgres',
@@ -218,21 +245,7 @@ is( $result, qq(1|2|22
 2|4|44
 3|6|66
 4|8|8
-5|10|10), 'add new table to existing publication');
-
-# TEST tab5: publisher-side col 'b' is not-generated and subscriber-side col 'b' is generated.
-# so confirmed that col 'b' IS NOT replicated and it will throw an error.
-# SUBSCRIPTION sub5 is created separately as sub5 will cause table sync worker to restart
-# repetitively
-my $offset = -s $node_subscriber->logfile;
-$node_subscriber->safe_psql('postgres',
-	"CREATE SUBSCRIPTION sub5 CONNECTION '$publisher_connstr' PUBLICATION pub5 WITH (include_generated_columns = true)"
-);
-$node_subscriber->wait_for_log(
-	qr/ERROR: ( [A-Z0-9]:)? logical replication target relation "public.tab5" has a generated column "b" but corresponding column on source relation is not a generated column/,
-	$offset);
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub5");
-
+5|10|10), 'after drop generated column expression');
 
 # try it with a subscriber-side trigger