PS_NITPICKS_20240718_GENCOLS_v190002.txt

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 935be7f..2e90d42 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -693,8 +693,8 @@ process_syncing_tables(XLogRecPtr current_lsn)
 }
 
 /*
- * Create list of columns for COPY based on logical relation mapping. Do not
- * include generated columns of the subscription table in the column list.
+ * Create list of columns for COPY based on logical relation mapping.
+ * Exclude columns that are subscription table generated columns.
  */
 static List *
 make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist)
@@ -707,7 +707,7 @@ make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist)
 	localgenlist = palloc0(rel->remoterel.natts * sizeof(bool));
 
 	/*
-	 * This loop checks for generated columns on subscription table.
+	 * This loop checks for generated columns of the subscription table.
 	 */
 	for (int i = 0; i < desc->natts; i++)
 	{
@@ -1010,15 +1010,12 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res,
 					 " WHERE a.attnum > 0::pg_catalog.int2"
 					 "   AND NOT a.attisdropped", lrel->remoteid);
 
-	if(server_version >= 120000)
+	if (server_version >= 120000)
 	{
 		bool gencols_allowed = server_version >= 180000 && MySubscription->includegencols;
 
-		if(!gencols_allowed)
-		{
-			/* Replication of generated cols is not supported. */
+		if (!gencols_allowed)
 			appendStringInfo(&cmd, " AND a.attgenerated = ''");
-		}
 	}
 
 	appendStringInfo(&cmd,
diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl
index 1814628..4537c6c 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -46,9 +46,9 @@ $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 20) STORED)");
 
 # 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
+# Publisher-side tab4 has generated cols 'b' and 'c'.
+# Subscriber-side tab4 has non-generated col 'b', and generated-col 'c'.
+# 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)"
 );
@@ -57,14 +57,14 @@ $node_subscriber->safe_psql('postgres',
 );
 
 # tab5:
-# publisher-side tab5 has non-generated col 'b' but
-# subscriber-side tab5 has generated col 'b'
+# Publisher-side tab5 has non-generated col 'b'.
+# 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)");
 
 # tab6:
-# tables for testing ALTER SUBSCRIPTION ... REFRESH PUBLICATION
+# Tables for testing ALTER SUBSCRIPTION ... 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)"
 );
@@ -73,8 +73,8 @@ $node_subscriber->safe_psql('postgres',
 );
 
 # tab7:
-# publisher-side tab7 has generated col 'b' but
-# subscriber-side tab7 do not have col 'b'
+# Publisher-side tab7 has generated col 'b'.
+# Subscriber-side tab7 does not have any col 'b'.
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab7 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)"
 );
@@ -209,9 +209,12 @@ is( $result, qq(1|21
 );
 
 #
-# 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 subscriber-side replicate data to correct columns.
+# TEST tab4:
+# Publisher-side tab4 has generated cols 'b' and 'c'.
+# Subscriber-side tab4 has non-generated col 'b', and generated-col 'c'.
+# Columns on publisher/subscriber are in a different order.
+#
+# Confirm despite the different order columns, they still replicate correctly.
 #
 $node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (4), (5)");
 $node_publisher->wait_for_catchup('sub4');
@@ -225,10 +228,15 @@ is( $result, qq(1|2|22
 5|10|110), 'replicate generated columns with different order on subscriber');
 
 #
-# 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.
+# TEST tab5:
+# Publisher-side tab5 has non-generated col 'b'.
+# Subscriber-side tab5 has generated col 'b'.
+#
+# Confirm that col 'b' is not replicated and it will throw an error.
+#
+# Note that 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',
@@ -254,9 +262,13 @@ is( $result, qq(1|2|22
 3|6|66), 'add new table to existing publication');
 
 #
-# TEST tab6: Drop the generated column's expression on subscriber side.
+# TEST tab6:
+# Drop the generated column's expression on subscriber side.
 # This changes the generated column into a non-generated column.
 #
+# Confirm that replication happens after the drop expression, because now we
+# are replicating from a generated column to a non-generated column.
+#
 $node_subscriber->safe_psql('postgres',
 	"ALTER TABLE tab6 ALTER COLUMN c DROP EXPRESSION");
 $node_publisher->safe_psql('postgres',
@@ -270,10 +282,16 @@ is( $result, qq(1|2|22
 5|10|10), 'after drop generated column expression');
 
 #
-# TEST tab7: publisher-side col 'b' is generated and subscriber-side do not have col 'b' and
-# 'include_generated_column' is 'true' so confirmed that col 'b' IS NOT replicated and
-# it will throw an error. The subscription sub7 is created here, instead of earlier with the
-# other subscriptions, because sub7 will cause the tablesync worker to restart repetitively.
+# TEST tab7, false
+# Publisher-side tab7 has generated col 'b'.
+# Subscriber-side tab7 does not have any col 'b'.
+# 'include_generated_columns' is true.
+#
+# Confirm that attempted replication of col 'b' will throw an error.
+#
+# Note the subscription sub7 is created here, instead of earlier with the
+# other subscriptions, because sub7 will cause the tablesync worker to restart
+# repetitively.
 #
 $node_subscriber->safe_psql('postgres',
 	"CREATE SUBSCRIPTION sub7 CONNECTION '$publisher_connstr' PUBLICATION pub7 with (include_generated_columns = true)"
@@ -284,8 +302,12 @@ $node_subscriber->wait_for_log(
 $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub7");
 
 #
-# TEST tab7: publisher-side col 'b' is generated and subscriber-side do not have col 'b' and
-# 'include_generated_column' is 'false' so confirmed that col 'b' IS NOT replicated.
+# TEST tab7:
+# Publisher-side tab7 has generated col 'b'.
+# Subscriber-side tab7 does not have any col 'b'.
+# 'include_generated_columns' is default (false).
+#
+# Confirm that col 'b' is not replicated, and no error occurs.
 #
 $node_subscriber->safe_psql('postgres',
 	"CREATE SUBSCRIPTION sub7 CONNECTION '$publisher_connstr' PUBLICATION pub7"