v18-0003-Fix-behaviour-for-Virtual-Generated-columns.patch

From 72488e973b1800570f54288250afceb3ee174e4a Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 16 Jul 2024 14:07:00 +0530
Subject: [PATCH v18 3/4] Fix behaviour for Virtual Generated columns

Currently during tablesync Virtual generated columns are also
replicated. Also during decoding a 'null' value appears for virtual
generated column. We are not supporting replication of virtual generated
columns for now. This patch fixes the behaviour for the same.

This patch has a dependency on Virtual Generated Columns
https://www.postgresql.org/message-id/flat/787a962749e7a822a44803ffbbdf021d8573ff53.camel%40post.pl#b64569231c9e1768e07f6bdc36c4070b
---
 .../expected/generated_columns.out            |  1 +
 .../test_decoding/sql/generated_columns.sql   |  4 +-
 contrib/test_decoding/test_decoding.c         | 15 ++++++-
 doc/src/sgml/protocol.sgml                    |  7 ++--
 doc/src/sgml/ref/create_subscription.sgml     |  4 +-
 src/backend/catalog/pg_publication.c          | 12 ++++++
 src/backend/replication/logical/proto.c       | 40 +++++++++++++++----
 src/backend/replication/logical/tablesync.c   |  8 +++-
 src/backend/replication/pgoutput/pgoutput.c   | 13 +++++-
 src/test/subscription/t/011_generated.pl      | 34 ++++++++--------
 10 files changed, 102 insertions(+), 36 deletions(-)

diff --git a/contrib/test_decoding/expected/generated_columns.out b/contrib/test_decoding/expected/generated_columns.out
index f3b26aa9e1..a79510705c 100644
--- a/contrib/test_decoding/expected/generated_columns.out
+++ b/contrib/test_decoding/expected/generated_columns.out
@@ -50,3 +50,4 @@ SELECT 'stop' FROM pg_drop_replication_slot('regression_slot');
  stop
 (1 row)
 
+-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns
diff --git a/contrib/test_decoding/sql/generated_columns.sql b/contrib/test_decoding/sql/generated_columns.sql
index 6d6d1d6564..997cdebc7e 100644
--- a/contrib/test_decoding/sql/generated_columns.sql
+++ b/contrib/test_decoding/sql/generated_columns.sql
@@ -19,4 +19,6 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
 
 DROP TABLE gencoltable;
 
-SELECT 'stop' FROM pg_drop_replication_slot('regression_slot');
\ No newline at end of file
+SELECT 'stop' FROM pg_drop_replication_slot('regression_slot');
+
+-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns
\ No newline at end of file
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c
index eaa3dbf9db..a847050f6e 100644
--- a/contrib/test_decoding/test_decoding.c
+++ b/contrib/test_decoding/test_decoding.c
@@ -557,8 +557,19 @@ tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple,
 		if (attr->attisdropped)
 			continue;
 
-		if (attr->attgenerated && !include_generated_columns)
-			continue;
+		if (attr->attgenerated)
+		{
+			/*
+			 * Don't print generated columns when
+			 * 'include_generated_columns' is false.
+			 */
+			if (!include_generated_columns)
+				continue;
+
+			/* Don't print generated columns unless they are STORED. */
+			if (attr->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		/*
 		 * Don't print system columns, oid will already have been printed if
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 226c3641b9..06554fb2af 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -3310,9 +3310,10 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
      <term>include_generated_columns</term>
       <listitem>
        <para>
-        Boolean option to enable generated columns. This option controls
-        whether generated columns should be included in the string
-        representation of tuples during logical decoding in PostgreSQL.
+        Boolean option to enable <literal>STORED</literal> generated columns.
+        This option controls whether <literal>STORED</literal> generated columns
+        should be included in the string representation of tuples during logical
+        decoding in PostgreSQL.
        </para>
       </listitem>
     </varlistentry>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 8fb4491b65..91e33174dc 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -433,8 +433,8 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
         <term><literal>include_generated_columns</literal> (<type>boolean</type>)</term>
         <listitem>
          <para>
-          Specifies whether the generated columns present in the tables
-          associated with the subscription should be replicated.
+          Specifies whether the <literal>STORED</literal> generated columns present
+          in the tables associated with the subscription should be replicated.
           The default is <literal>false</literal>.
          </para>
          <para>
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index f611148472..1809e140ea 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -506,6 +506,7 @@ publication_translate_columns(Relation targetrel, List *columns,
 	Bitmapset  *set = NULL;
 	ListCell   *lc;
 	int			n = 0;
+	TupleDesc	tupdesc = RelationGetDescr(targetrel);
 
 	/* Bail out when no column list defined. */
 	if (!columns)
@@ -520,6 +521,7 @@ publication_translate_columns(Relation targetrel, List *columns,
 	{
 		char	   *colname = strVal(lfirst(lc));
 		AttrNumber	attnum = get_attnum(RelationGetRelid(targetrel), colname);
+		Form_pg_attribute att;
 
 		if (attnum == InvalidAttrNumber)
 			ereport(ERROR,
@@ -533,6 +535,13 @@ publication_translate_columns(Relation targetrel, List *columns,
 					errmsg("cannot use system column \"%s\" in publication column list",
 						   colname));
 
+		att = TupleDescAttr(tupdesc, attnum - 1);
+		if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+			ereport(ERROR,
+					errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+					errmsg("generated column \"%s\" is not STORED so cannot be used in a publication column list",
+						   colname));
+
 		if (bms_is_member(attnum, set))
 			ereport(ERROR,
 					errcode(ERRCODE_DUPLICATE_OBJECT),
@@ -1228,6 +1237,9 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
 				if (att->attisdropped)
 					continue;
 
+				if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+					continue;
+
 				attnums[nattnums++] = att->attnum;
 			}
 
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index e694baca0a..cad1b76e7a 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -793,8 +793,14 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot,
 		if (att->attisdropped)
 			continue;
 
-		if (att->attgenerated && !include_generated_columns)
-			continue;
+		if (att->attgenerated)
+		{
+			if (!include_generated_columns)
+				continue;
+
+			if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		if (!column_in_column_list(att->attnum, columns))
 			continue;
@@ -817,8 +823,14 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot,
 		if (att->attisdropped)
 			continue;
 
-		if (att->attgenerated && !include_generated_columns)
-			continue;
+		if (att->attgenerated)
+		{
+			if (!include_generated_columns)
+				continue;
+
+			if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		if (!column_in_column_list(att->attnum, columns))
 			continue;
@@ -957,8 +969,14 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns,
 		if (att->attisdropped)
 			continue;
 
-		if (att->attgenerated && !include_generated_columns)
-			continue;
+		if (att->attgenerated)
+		{
+			if (!include_generated_columns)
+				continue;
+
+			if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		if (!column_in_column_list(att->attnum, columns))
 			continue;
@@ -981,8 +999,14 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns,
 		if (att->attisdropped)
 			continue;
 
-		if (att->attgenerated && !include_generated_columns)
-			continue;
+		if (att->attgenerated)
+		{
+			if (!include_generated_columns)
+				continue;
+
+			if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		if (!column_in_column_list(att->attnum, columns))
 			continue;
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 935be7f934..b1407cc97d 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -1014,7 +1014,13 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res,
 	{
 		bool gencols_allowed = server_version >= 180000 && MySubscription->includegencols;
 
-		if(!gencols_allowed)
+		if (gencols_allowed)
+		{
+			/* Replication of generated cols is supported, but not VIRTUAL cols. */
+			/* TODO: use ATTRIBUTE_GENERATED_VIRTUAL*/
+			appendStringInfo(&cmd, " AND a.attgenerated != 'v'");
+		}
+		else
 		{
 			/* Replication of generated cols is not supported. */
 			appendStringInfo(&cmd, " AND a.attgenerated = ''");
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 6bc9f9d403..a256ab7262 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -786,8 +786,14 @@ send_relation_and_attrs(Relation relation, TransactionId xid,
 		if (att->attisdropped)
 			continue;
 
-		if (att->attgenerated && !include_generated_columns)
-			continue;
+		if (att->attgenerated)
+		{
+			if (!include_generated_columns)
+				continue;
+
+			if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+				continue;
+		}
 
 		if (att->atttypid < FirstGenbkiObjectId)
 			continue;
@@ -1108,6 +1114,9 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 						if (att->attisdropped)
 							continue;
 
+						if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED)
+							continue;
+
 						nliveatts++;
 					}
 
diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl
index d4327717cc..72c0d061c2 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -30,22 +30,22 @@ $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 22) STORED, c int)"
 );
 
-# publisher-side tab2 has generated col 'b' but subscriber-side tab2 has NON-generated col 'b'.
+# publisher-side tab2 has stored generated col 'b' but subscriber-side tab2 has NON-generated col 'b'.
 $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)");
 
 # tab3:
-# publisher-side tab3 has generated col 'b' but
-# subscriber-side tab3 has DIFFERENT COMPUTATION generated col 'b'.
+# publisher-side tab3 has stored generated col 'b' but
+# subscriber-side tab3 has DIFFERENT COMPUTATION stored 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'
+# publisher-side tab4 has stored generated cols 'b' and 'c' but
+# subscriber-side tab4 has non-generated col 'b', and stored 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)"
@@ -56,7 +56,7 @@ $node_subscriber->safe_psql('postgres',
 
 # tab5:
 # publisher-side tab5 has non-generated col 'b' but
-# subscriber-side tab5 has generated col 'b'
+# subscriber-side tab5 has stored 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)");
@@ -71,7 +71,7 @@ $node_subscriber->safe_psql('postgres',
 );
 
 # tab7:
-# publisher-side tab7 has generated col 'b' but
+# publisher-side tab7 has stored generated col 'b' but
 # subscriber-side tab7 do not have col 'b'
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab7 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)"
@@ -167,7 +167,7 @@ is( $result, qq(1|22|
 6|132|), 'generated columns replicated');
 
 #
-# TEST tab2: the publisher-side col 'b' is generated, and the subscriber-side
+# TEST tab2: the publisher-side col 'b' is stored generated, and the subscriber-side
 # col 'b' is not generated, so confirm that col 'b' IS replicated.
 #
 $node_publisher->safe_psql('postgres', "INSERT INTO tab2 VALUES (4), (5)");
@@ -183,8 +183,8 @@ is( $result, qq(1|2
 );
 
 #
-# TEST tab3: the publisher-side col 'b' is generated, and the subscriber-side
-# col 'b' is also generated, so confirmed that col 'b' IS NOT replicated. We
+# TEST tab3: the publisher-side col 'b' is stored generated, and the subscriber-side
+# col 'b' is also stored generated, so confirmed that col 'b' IS NOT replicated. We
 # can know this because the result value is the subscriber-side computation
 # (which is not the same as the publisher-side computation for col 'b').
 #
@@ -201,8 +201,8 @@ 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
+# TEST tab4: the publisher-side cols 'b' and 'c' are stored generated and subscriber-side
+# col 'b' is not generated and col 'c' is stored 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)");
@@ -218,7 +218,7 @@ is( $result, qq(1|2|22
 
 #
 # 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.
+# is stored 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.
 #
@@ -246,8 +246,8 @@ 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.
-# This changes the generated column into a non-generated column.
+# TEST tab6: Drop the stored generated column's expression on subscriber side.
+# This changes the stored generated column into a non-generated column.
 #
 $node_subscriber->safe_psql('postgres',
 	"ALTER TABLE tab6 ALTER COLUMN c DROP EXPRESSION");
@@ -262,7 +262,7 @@ 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
+# TEST tab7: publisher-side col 'b' is stored 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.
@@ -276,7 +276,7 @@ $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
+# TEST tab7: publisher-side col 'b' is stored generated and subscriber-side do not have col 'b' and
 # 'include_generated_column' is 'false' so confirmed that col 'b' IS NOT replicated.
 #
 $node_subscriber->safe_psql('postgres',
-- 
2.34.1