v16-0002-Support-replication-of-generated-column-during-i.patch

From 67a3681edc2f25961cc1c8ee6084d0e137ba132a Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Fri, 5 Jul 2024 15:37:44 +0530
Subject: [PATCH v16 2/3] Support replication of generated column during
 initial sync

When 'copy_data' is true, during the initial sync, the data is replicated from
the publisher to the subscriber using the COPY command. The normal COPY
command does not copy generated columns, so when 'include_generated_columns'
is true, we need to copy using the syntax:
'COPY (SELECT column_name FROM table_name) TO STDOUT'.

Summary:

when (include_generated_columns = true)

* publisher not-generated column => subscriber not-generated column:
This is just normal logical replication (not changed by this patch).

* publisher not-generated column => subscriber generated column: This
will give ERROR.

* publisher generated column => subscriber not-generated column: The
publisher generated column value is copied.

* publisher generated column => subscriber generated column: The
publisher generated column value is not copied. The subscriber
generated column will be filled with the subscriber-side computed or
default data.

when (include_generated_columns = false)

* publisher not-generated column => subscriber not-generated column:
This is just normal logical replication (not changed by this patch).

* publisher not-generated column => subscriber generated column: This
will give ERROR.

* publisher generated column => subscriber not-generated column:
Publisher generated column is not replicated. The subscriber column
will be filled with the subscriber-side default data.

* publisher generated column => subscriber generated column: Publisher
generated column is not replicated. The subscriber generated column
will be filed with the subscriber-side computed or default data.
---
 doc/src/sgml/ref/create_subscription.sgml   |   4 -
 src/backend/commands/subscriptioncmds.c     |  14 --
 src/backend/replication/logical/relation.c  |   2 +-
 src/backend/replication/logical/tablesync.c | 135 +++++++++++++++----
 src/include/replication/logicalrelation.h   |   3 +-
 src/test/regress/expected/subscription.out  |   3 -
 src/test/regress/sql/subscription.sql       |   3 -
 src/test/subscription/t/004_sync.pl         |  56 ++++++++
 src/test/subscription/t/011_generated.pl    | 139 +++++++++++++++++++-
 9 files changed, 303 insertions(+), 56 deletions(-)

diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index ee27a5873a..8fb4491b65 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -442,10 +442,6 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
           has no effect; the subscriber column will be filled as normal with the
           subscriber-side computed or default data.
          </para>
-         <para>
-         This parameter can only be set <literal>true</literal> if <literal>copy_data</literal> is
-         set to <literal>false</literal>.
-         </para>
         </listitem>
        </varlistentry>
       </variablelist></para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index f942b58565..408a9157ec 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -459,20 +459,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
 								"slot_name = NONE", "create_slot = false")));
 		}
 	}
-
-	/*
-	 * Do additional checking for disallowed combination when copy_data and
-	 * include_generated_columns are true. COPY of generated columns is not
-	 * supported yet.
-	 */
-	if (opts->copy_data && opts->include_generated_columns)
-	{
-		ereport(ERROR,
-				errcode(ERRCODE_SYNTAX_ERROR),
-		/*- translator: both %s are strings of the form "option = value" */
-				errmsg("%s and %s are mutually exclusive options",
-					   "copy_data = true", "include_generated_columns = true"));
-	}
 }
 
 /*
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 5de1531567..9de0b75330 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -205,7 +205,7 @@ logicalrep_relmap_update(LogicalRepRelation *remoterel)
  *
  * Returns -1 if not found.
  */
-static int
+int
 logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname)
 {
 	int			i;
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index b00267f042..1edba12a36 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -118,6 +118,7 @@
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
+#include "utils/rel.h"
 #include "utils/rls.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
@@ -692,21 +693,68 @@ process_syncing_tables(XLogRecPtr current_lsn)
 }
 
 /*
- * Create list of columns for COPY based on logical relation mapping.
+ * Create list of columns for COPY based on logical relation mapping. Do not
+ * include generated columns of the subscription table in the column list.
  */
 static List *
-make_copy_attnamelist(LogicalRepRelMapEntry *rel)
+make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist)
 {
 	List	   *attnamelist = NIL;
-	int			i;
+	bool	   *localgenlist;
+	TupleDesc	desc;
 
-	for (i = 0; i < rel->remoterel.natts; i++)
+	desc = RelationGetDescr(rel->localrel);
+	localgenlist = palloc0(rel->remoterel.natts * sizeof(bool));
+
+	/*
+	 * This loop checks for generated columns on subscription table.
+	 */
+	for (int i = 0; i < desc->natts; i++)
 	{
-		attnamelist = lappend(attnamelist,
-							  makeString(rel->remoterel.attnames[i]));
+		int			remote_attnum;
+		Form_pg_attribute attr = TupleDescAttr(desc, i);
+
+		if (!attr->attgenerated)
+			continue;
+
+		remote_attnum = logicalrep_rel_att_by_name(&rel->remoterel,
+											NameStr(attr->attname));
+
+		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[remote_attnum])
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("logical replication target relation \"%s.%s\" has a generated column \"%s\" "
+								"but corresponding column on source relation is not a generated column",
+						 rel->remoterel.nspname, rel->remoterel.relname, NameStr(attr->attname))));
+
+			/*
+			 * 'localgenlist' records that this is a generated column in
+			 * the subscription table. Later, we use this information to
+			 * skip adding this column to the column list for COPY.
+			 */
+			localgenlist[remote_attnum] = true;
+		}
 	}
 
+	/*
+	 * Construct column list for COPY, excluding columns that are
+	 * subscription table generated columns.
+	 */
+	for (int i = 0; i < rel->remoterel.natts; i++)
+	{
+		if (!localgenlist[i])
+			attnamelist = lappend(attnamelist,
+								  makeString(rel->remoterel.attnames[i]));
+	}
 
+	pfree(localgenlist);
 	return attnamelist;
 }
 
@@ -791,19 +839,21 @@ copy_read_data(void *outbuf, int minread, int maxread)
  * qualifications to be used in the COPY command.
  */
 static void
-fetch_remote_table_info(char *nspname, char *relname,
+fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res,
 						LogicalRepRelation *lrel, List **qual)
 {
 	WalRcvExecResult *res;
 	StringInfoData cmd;
 	TupleTableSlot *slot;
 	Oid			tableRow[] = {OIDOID, CHAROID, CHAROID};
-	Oid			attrRow[] = {INT2OID, TEXTOID, OIDOID, BOOLOID};
+	Oid			attrRow[] = {INT2OID, TEXTOID, OIDOID, BOOLOID, BOOLOID};
 	Oid			qualRow[] = {TEXTOID};
 	bool		isnull;
+	bool	   *remotegenlist;
 	int			natt;
 	ListCell   *lc;
 	Bitmapset  *included_cols = NULL;
+	int			server_version = walrcv_server_version(LogRepWorkerWalRcvConn);
 
 	lrel->nspname = nspname;
 	lrel->relname = relname;
@@ -851,7 +901,7 @@ fetch_remote_table_info(char *nspname, char *relname,
 	 * We need to do this before fetching info about column names and types,
 	 * so that we can skip columns that should not be replicated.
 	 */
-	if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
+	if (server_version >= 150000)
 	{
 		WalRcvExecResult *pubres;
 		TupleTableSlot *tslot;
@@ -948,18 +998,27 @@ fetch_remote_table_info(char *nspname, char *relname,
 					 "SELECT a.attnum,"
 					 "       a.attname,"
 					 "       a.atttypid,"
-					 "       a.attnum = ANY(i.indkey)"
+					 "       a.attnum = ANY(i.indkey)");
+
+	if(server_version >= 120000)
+		appendStringInfo(&cmd, ", a.attgenerated != ''");
+
+	appendStringInfo(&cmd,
 					 "  FROM pg_catalog.pg_attribute a"
 					 "  LEFT JOIN pg_catalog.pg_index i"
 					 "       ON (i.indexrelid = pg_get_replica_identity_index(%u))"
 					 " WHERE a.attnum > 0::pg_catalog.int2"
-					 "   AND NOT a.attisdropped %s"
+					 "   AND NOT a.attisdropped", lrel->remoteid);
+
+	if ((server_version >= 120000 && server_version < 180000) ||
+		!MySubscription->includegencols)
+			appendStringInfo(&cmd, " AND a.attgenerated = ''");
+
+	appendStringInfo(&cmd,
 					 "   AND a.attrelid = %u"
 					 " ORDER BY a.attnum",
-					 lrel->remoteid,
-					 (walrcv_server_version(LogRepWorkerWalRcvConn) >= 120000 ?
-					  "AND a.attgenerated = ''" : ""),
 					 lrel->remoteid);
+
 	res = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data,
 					  lengthof(attrRow), attrRow);
 
@@ -973,6 +1032,7 @@ fetch_remote_table_info(char *nspname, char *relname,
 	lrel->attnames = palloc0(MaxTupleAttributeNumber * sizeof(char *));
 	lrel->atttyps = palloc0(MaxTupleAttributeNumber * sizeof(Oid));
 	lrel->attkeys = NULL;
+	remotegenlist = palloc0(MaxTupleAttributeNumber * sizeof(bool));
 
 	/*
 	 * Store the columns as a list of names.  Ignore those that are not
@@ -1005,6 +1065,8 @@ fetch_remote_table_info(char *nspname, char *relname,
 		if (DatumGetBool(slot_getattr(slot, 4, &isnull)))
 			lrel->attkeys = bms_add_member(lrel->attkeys, natt);
 
+		remotegenlist[natt] = DatumGetBool(slot_getattr(slot, 5, &isnull));
+
 		/* Should never happen. */
 		if (++natt >= MaxTupleAttributeNumber)
 			elog(ERROR, "too many columns in remote table \"%s.%s\"",
@@ -1015,7 +1077,7 @@ fetch_remote_table_info(char *nspname, char *relname,
 	ExecDropSingleTupleTableSlot(slot);
 
 	lrel->natts = natt;
-
+	*remotegenlist_res = remotegenlist;
 	walrcv_clear_result(res);
 
 	/*
@@ -1037,7 +1099,7 @@ fetch_remote_table_info(char *nspname, char *relname,
 	 * 3) one of the subscribed publications is declared as TABLES IN SCHEMA
 	 * that includes this relation
 	 */
-	if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
+	if (server_version >= 150000)
 	{
 		StringInfoData pub_names;
 
@@ -1123,10 +1185,13 @@ copy_table(Relation rel)
 	List	   *attnamelist;
 	ParseState *pstate;
 	List	   *options = NIL;
+	bool 	   *remotegenlist;
+	bool		gencol_copy_needed  = false;
 
 	/* Get the publisher relation info. */
 	fetch_remote_table_info(get_namespace_name(RelationGetNamespace(rel)),
-							RelationGetRelationName(rel), &lrel, &qual);
+							RelationGetRelationName(rel), &remotegenlist,
+							&lrel, &qual);
 
 	/* Put the relation into relmap. */
 	logicalrep_relmap_update(&lrel);
@@ -1135,11 +1200,29 @@ copy_table(Relation rel)
 	relmapentry = logicalrep_rel_open(lrel.remoteid, NoLock);
 	Assert(rel == relmapentry->localrel);
 
+	attnamelist = make_copy_attnamelist(relmapentry, remotegenlist);
+
 	/* Start copy on the publisher. */
 	initStringInfo(&cmd);
 
-	/* Regular table with no row filter */
-	if (lrel.relkind == RELKIND_RELATION && qual == NIL)
+	/* Check if remote column list has any generated column */
+	if(MySubscription->includegencols)
+	{
+		for (int i = 0; i < relmapentry->remoterel.natts; i++)
+		{
+			if(remotegenlist[i])
+			{
+				gencol_copy_needed = true;
+				break;
+			}
+		}
+	}
+
+	/*
+	 * Regular table with no row filter and copy of generated columns is
+	 * not necessary.
+	 */
+	if (lrel.relkind == RELKIND_RELATION && qual == NIL && !gencol_copy_needed)
 	{
 		appendStringInfo(&cmd, "COPY %s",
 						 quote_qualified_identifier(lrel.nspname, lrel.relname));
@@ -1173,13 +1256,20 @@ copy_table(Relation rel)
 		 * (SELECT ...), but we can't just do SELECT * because we need to not
 		 * copy generated columns. For tables with any row filters, build a
 		 * 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 and the remote
+		 * table has generated columns, because copy of generated columns is
+		 * not supported by the normal COPY.
 		 */
+		int i = 0;
+
 		appendStringInfoString(&cmd, "COPY (SELECT ");
-		for (int i = 0; i < lrel.natts; i++)
+		foreach_node(String, att_name, attnamelist)
 		{
-			appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i]));
-			if (i < lrel.natts - 1)
+			if (i++)
 				appendStringInfoString(&cmd, ", ");
+			appendStringInfoString(&cmd, quote_identifier(strVal(att_name)));
 		}
 
 		appendStringInfoString(&cmd, " FROM ");
@@ -1237,7 +1327,6 @@ copy_table(Relation rel)
 	(void) addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
 										 NULL, false, false);
 
-	attnamelist = make_copy_attnamelist(relmapentry);
 	cstate = BeginCopyFrom(pstate, rel, NULL, NULL, false, copy_read_data, attnamelist, options);
 
 	/* Do the copy */
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index e687b40a56..797e66dfdb 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -41,7 +41,8 @@ typedef struct LogicalRepRelMapEntry
 
 extern void logicalrep_relmap_update(LogicalRepRelation *remoterel);
 extern void logicalrep_partmap_reset_relmap(LogicalRepRelation *remoterel);
-
+extern int logicalrep_rel_att_by_name(LogicalRepRelation *remoterel,
+									  const char *attname);
 extern LogicalRepRelMapEntry *logicalrep_rel_open(LogicalRepRelId remoteid,
 												  LOCKMODE lockmode);
 extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *root,
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 36916c0ac2..592c1f0667 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -99,9 +99,6 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU
 ERROR:  subscription with slot_name = NONE must also set create_slot = false
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
 ERROR:  subscription with slot_name = NONE must also set enabled = false
--- fail - include_generated_columns and copy_data = true are mutually exclusive
-CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (include_generated_columns = true);
-ERROR:  copy_data = true and include_generated_columns = true are mutually exclusive options
 -- fail - include_generated_columns must be boolean
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, include_generated_columns = foo);
 ERROR:  include_generated_columns requires a Boolean value
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 7944152124..8c7381fbfc 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -60,9 +60,6 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
 
--- fail - include_generated_columns and copy_data = true are mutually exclusive
-CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (include_generated_columns = true);
-
 -- fail - include_generated_columns must be boolean
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, include_generated_columns = foo);
 
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index a2d9462395..62462c0c95 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -172,6 +172,62 @@ ok( $node_publisher->poll_query_until(
 		'postgres', 'SELECT count(*) = 0 FROM pg_replication_slots'),
 	'DROP SUBSCRIPTION during error can clean up the slots on the publisher');
 
+# clean up
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_rep");
+$node_subscriber->safe_psql('postgres', "DROP TABLE tab_rep");
+
+#
+# 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; note column 'b' is missing
+$node_subscriber->safe_psql('postgres', "CREATE TABLE tab_rep (a int)");
+
+my $offset = -s $node_subscriber->logfile;
+
+# create the subscription
+$node_subscriber->safe_psql('postgres',
+	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
+);
+
+# check for missing column error
+$node_subscriber->wait_for_log(
+	qr/ERROR: ( [A-Z0-9]+:)? logical replication target relation "public.tab_rep" is missing replicated column: "b"/,
+	$offset);
+
+# clean up
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
+$node_subscriber->safe_psql('postgres', "DROP TABLE tab_rep");
+
+#
+# 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 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);
+
 $node_subscriber->stop('fast');
 $node_publisher->stop('fast');
 
diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl
index 25edc6fa17..9e26373c43 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -16,6 +16,8 @@ $node_publisher->start;
 
 my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber');
 $node_subscriber->init;
+$node_subscriber->append_conf('postgresql.conf',
+	"max_logical_replication_workers = 10");
 $node_subscriber->start;
 
 my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
@@ -33,12 +35,41 @@ $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)");
 
+# 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)"
+);
+
+# 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)");
+
+# tab6:
+# 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)"
+);
+$node_subscriber->safe_psql('postgres',
+	"CREATE TABLE tab6 (a int, b int, c int GENERATED ALWAYS AS (a * 22) STORED)"
+);
+
 # data for initial sync
 
 $node_publisher->safe_psql('postgres',
@@ -47,6 +78,12 @@ $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab2 (a) VALUES (1), (2), (3)");
 $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab3 (a) VALUES (1), (2), (3)");
+$node_publisher->safe_psql('postgres',
+	"INSERT INTO tab4 (a) VALUES (1), (2), (3)");
+$node_publisher->safe_psql('postgres',
+	"INSERT INTO tab5 (a, b) VALUES (1, 1), (2, 2), (3, 3)");
+$node_publisher->safe_psql('postgres',
+	"INSERT INTO tab6 (a) VALUES (1), (2), (3)");
 
 $node_publisher->safe_psql('postgres',
 	"CREATE PUBLICATION pub1 FOR TABLE tab1");
@@ -54,15 +91,22 @@ $node_publisher->safe_psql('postgres',
 	"CREATE PUBLICATION pub2 FOR TABLE tab2");
 $node_publisher->safe_psql('postgres',
 	"CREATE PUBLICATION pub3 FOR TABLE tab3");
+$node_publisher->safe_psql('postgres',
+	"CREATE PUBLICATION pub4 FOR TABLE tab4");
+$node_publisher->safe_psql('postgres',
+	"CREATE PUBLICATION pub5 FOR TABLE tab5");
 
 $node_subscriber->safe_psql('postgres',
 	"CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub1"
 );
 $node_subscriber->safe_psql('postgres',
-	"CREATE SUBSCRIPTION sub2 CONNECTION '$publisher_connstr' PUBLICATION pub2 WITH (include_generated_columns = true, copy_data = false)"
+	"CREATE SUBSCRIPTION sub2 CONNECTION '$publisher_connstr' PUBLICATION pub2 WITH (include_generated_columns = true)"
 );
 $node_subscriber->safe_psql('postgres',
-	"CREATE SUBSCRIPTION sub3 CONNECTION '$publisher_connstr' PUBLICATION pub3 WITH (include_generated_columns = true, copy_data = false)"
+	"CREATE SUBSCRIPTION sub3 CONNECTION '$publisher_connstr' PUBLICATION pub3 WITH (include_generated_columns = true)"
+);
+$node_subscriber->safe_psql('postgres',
+	"CREATE SUBSCRIPTION sub4 CONNECTION '$publisher_connstr' PUBLICATION pub4 WITH (include_generated_columns = true)"
 );
 
 # Wait for initial sync of all subscriptions
@@ -74,10 +118,24 @@ is( $result, qq(1|22
 3|66), 'generated columns initial sync');
 
 $result = $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab2");
-is($result, qq(), 'generated columns initial sync');
+is( $result, qq(1|2
+2|4
+3|6), 'generated columns initial sync');
 
 $result = $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab3");
-is($result, qq(), 'generated columns initial sync');
+is( $result, qq(1|21
+2|22
+3|23), 'generated columns initial sync');
+
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b, c FROM tab4 ORDER BY a");
+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
 
@@ -102,7 +160,10 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab2 VALUES (4), (5)");
 $node_publisher->wait_for_catchup('sub2');
 $result =
   $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab2 ORDER BY a");
-is( $result, qq(4|8
+is( $result, qq(1|2
+2|4
+3|6
+4|8
 5|10),
 	'confirm generated columns ARE replicated when the subscriber-side column is not generated'
 );
@@ -117,11 +178,75 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab3 VALUES (4), (5)");
 $node_publisher->wait_for_catchup('sub3');
 $result =
   $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab3 ORDER BY a");
-is( $result, qq(4|24
+is( $result, qq(1|21
+2|22
+3|23
+4|24
 5|25),
 	'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 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 =
+  $node_subscriber->safe_psql('postgres',
+	"SELECT a, b, c FROM tab4 ORDER BY a");
+is( $result, qq(1|2|22
+2|4|44
+3|6|66
+4|8|88
+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.
+#
+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");
+$node_subscriber->safe_psql('postgres',
+	"ALTER SUBSCRIPTION sub4 REFRESH PUBLICATION");
+$node_publisher->wait_for_catchup('sub4');
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b, c FROM tab6 ORDER BY a");
+is( $result, qq(1|2|22
+2|4|44
+3|6|66), 'add new table to existing publication');
+
+#
+# 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',
+	"INSERT INTO tab6 (a) VALUES (4), (5)");
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT a, b, c FROM tab6 ORDER BY a");
+is( $result, qq(1|2|22
+2|4|44
+3|6|66
+4|8|8
+5|10|10), 'after drop generated column expression');
+
 # try it with a subscriber-side trigger
 
 $node_subscriber->safe_psql(
-- 
2.34.1