PS_NITPICKS_20240930_V340001.txt

text/plain

Filename: PS_NITPICKS_20240930_V340001.txt
Type: text/plain
Part: 0
Message: Re: Pgoutput not capturing the generated columns
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 24c56ed..da7ebe9 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1009,8 +1009,9 @@ pgoutput_row_filter_init(PGOutputData *data, List *publications,
 }
 
 /*
- * Prepare new column list bitmap.
- * This encompasses all table columns, excluding the generated ones.
+ * Return a column list bitmap for the specified table.
+ *
+ * Generated columns are excluded.
  */
 static Bitmapset *
 prepare_all_columns_bms(PGOutputData *data, RelationSyncEntry *entry,
@@ -1079,7 +1080,7 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 		 */
 		if (!pub->alltables || !pub->pubgencols)
 		{
-			bool		pub_rel_has_collist = true;
+			bool		pub_rel_has_collist = false;
 
 			/*
 			 * Check for the presence of a column list in this publication.
@@ -1094,28 +1095,31 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 
 			if (HeapTupleIsValid(cftuple))
 			{
+				bool pub_no_list = true;
+
 				/* Lookup the column list attribute. */
 				cfdatum = SysCacheGetAttr(PUBLICATIONRELMAP, cftuple,
 										  Anum_pg_publication_rel_prattrs,
-										  &pub_rel_has_collist);
+										  &pub_no_list);
+
+				pub_rel_has_collist = !pub_no_list;
 			}
 
 			/* Build the column list bitmap in the per-entry context. */
-			if (!pub_rel_has_collist || !pub->pubgencols)
+			if (pub_rel_has_collist || !pub->pubgencols)
 			{
-				int			i;
 				int			nliveatts = 0;
 				TupleDesc	desc = RelationGetDescr(relation);
 
 				pgoutput_ensure_entry_cxt(data, entry);
 
-				if (!pub_rel_has_collist)
+				if (pub_rel_has_collist)
 					cols = pub_collist_to_bitmapset(cols, cfdatum, entry->entry_cxt);
 				else
 					cols = prepare_all_columns_bms(data, entry, desc);
 
 				/* Get the number of live attributes. */
-				for (i = 0; i < desc->natts; i++)
+				for (int i = 0; i < desc->natts; i++)
 				{
 					Form_pg_attribute att = TupleDescAttr(desc, i);
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a0dad1e..ec12fd9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4304,19 +4304,19 @@ getPublications(Archive *fout)
 		appendPQExpBufferStr(query,
 							 "SELECT p.tableoid, p.oid, p.pubname, "
 							 "p.pubowner, "
-							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, p.pubviaroot, false AS pubviagencols "
+							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, p.pubviaroot, false AS pubgencols "
 							 "FROM pg_publication p");
 	else if (fout->remoteVersion >= 110000)
 		appendPQExpBufferStr(query,
 							 "SELECT p.tableoid, p.oid, p.pubname, "
 							 "p.pubowner, "
-							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, false AS pubviaroot, false AS pubviagencols "
+							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, false AS pubviaroot, false AS pubgencols "
 							 "FROM pg_publication p");
 	else
 		appendPQExpBufferStr(query,
 							 "SELECT p.tableoid, p.oid, p.pubname, "
 							 "p.pubowner, "
-							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, false AS pubtruncate, false AS pubviaroot, false AS pubviagencols "
+							 "p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete, false AS pubtruncate, false AS pubviaroot, false AS pubgencols "
 							 "FROM pg_publication p");
 
 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index a26b5a4..a532cd6 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -693,7 +693,7 @@ UPDATE testpub_tbl5 SET a = 1;
 ERROR:  cannot update table "testpub_tbl5"
 DETAIL:  Column list used by the publication does not cover the replica identity.
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
--- ok: generated columns can be in the list too
+-- ok: generated column "d" can be in the list too
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d);
 WARNING:  specified generated column "d" in publication column list when publish_generated_columns as false
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
@@ -1756,6 +1756,7 @@ DROP PUBLICATION pub;
 DROP TABLE sch1.tbl1;
 DROP SCHEMA sch1 cascade;
 DROP SCHEMA sch2 cascade;
+-- ======================================================
 -- Test the publication 'publish_generated_columns' parameter enabled or disabled
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_generated_columns=1);
@@ -1777,30 +1778,29 @@ CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_generated_columns=0);
 RESET client_min_messages;
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
--- Test the publication 'publish_generated_columns' parameter enabled or disabled for different combinations.
+-- Test the 'publish_generated_columns' parameter enabled or disabled for
+-- different scenarios with/without generated columns in column lists.
 SET client_min_messages = 'WARNING';
 CREATE TABLE gencols (a int, gen1 int GENERATED ALWAYS AS (a * 2) STORED);
--- gencols in column list with 'publish_generated_columns'=false.
+-- gencols in column list with 'publish_generated_columns'=false
 CREATE PUBLICATION pub1 FOR table gencols(a, gen1) WITH (publish_generated_columns=false);
 WARNING:  specified generated column "gen1" in publication column list when publish_generated_columns as false
 WARNING:  "wal_level" is insufficient to publish logical changes
 HINT:  Set "wal_level" to "logical" before creating subscriptions.
--- gencols in column list with 'publish_generated_columns'=true.
+-- gencols in column list with 'publish_generated_columns'=true
 CREATE PUBLICATION pub2 FOR table gencols(a, gen1) WITH (publish_generated_columns=true);
 WARNING:  "wal_level" is insufficient to publish logical changes
 HINT:  Set "wal_level" to "logical" before creating subscriptions.
--- ALTER PUBLICATION setting 'publication_generate_columns'=false.
+-- gencols in column list, then set 'publication_generate_columns'=false
 ALTER PUBLICATION pub2 SET (publish_generated_columns = false);
--- No gencols in column list with 'publish_generated_columns'=false.
-CREATE PUBLICATION pub3 WITH (publish_generated_columns=false);
-WARNING:  "wal_level" is insufficient to publish logical changes
-HINT:  Set "wal_level" to "logical" before creating subscriptions.
--- ALTER PUBLICATION to add gencols to column list.
-ALTER PUBLICATION pub3 ADD TABLE gencols(a, gen1);
+-- remove gencols from column list, when 'publish_generated_columns'=false
+ALTER PUBLICATION pub2 SET TABLE gencols(a);
+-- Add gencols in column list, when 'publish_generated_columns'=false.
+ALTER PUBLICATION pub2 SET TABLE gencols(a, gen1);
+WARNING:  specified generated column "gen1" in publication column list when publish_generated_columns as false
 WARNING:  specified generated column "gen1" in publication column list when publish_generated_columns as false
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
-DROP PUBLICATION pub3;
 DROP TABLE gencols;
 RESET client_min_messages;
 RESET SESSION AUTHORIZATION;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index d7b43dc..6a74fd6 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -415,7 +415,7 @@ ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, x);
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (b, c);
 UPDATE testpub_tbl5 SET a = 1;
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
--- ok: generated columns can be in the list too
+-- ok: generated column "d" can be in the list too
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d);
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
 -- error: system attributes "ctid" not allowed in column list
@@ -1112,6 +1112,7 @@ DROP PUBLICATION pub;
 DROP TABLE sch1.tbl1;
 DROP SCHEMA sch1 cascade;
 DROP SCHEMA sch2 cascade;
+-- ======================================================
 
 -- Test the publication 'publish_generated_columns' parameter enabled or disabled
 SET client_min_messages = 'ERROR';
@@ -1125,24 +1126,28 @@ RESET client_min_messages;
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
 
--- Test the publication 'publish_generated_columns' parameter enabled or disabled for different combinations.
+-- Test the 'publish_generated_columns' parameter enabled or disabled for
+-- different scenarios with/without generated columns in column lists.
 SET client_min_messages = 'WARNING';
 CREATE TABLE gencols (a int, gen1 int GENERATED ALWAYS AS (a * 2) STORED);
--- gencols in column list with 'publish_generated_columns'=false.
+
+-- gencols in column list with 'publish_generated_columns'=false
 CREATE PUBLICATION pub1 FOR table gencols(a, gen1) WITH (publish_generated_columns=false);
--- gencols in column list with 'publish_generated_columns'=true.
+
+-- gencols in column list with 'publish_generated_columns'=true
 CREATE PUBLICATION pub2 FOR table gencols(a, gen1) WITH (publish_generated_columns=true);
--- ALTER PUBLICATION setting 'publication_generate_columns'=false.
+
+-- gencols in column list, then set 'publication_generate_columns'=false
 ALTER PUBLICATION pub2 SET (publish_generated_columns = false);
 
--- No gencols in column list with 'publish_generated_columns'=false.
-CREATE PUBLICATION pub3 WITH (publish_generated_columns=false);
--- ALTER PUBLICATION to add gencols to column list.
-ALTER PUBLICATION pub3 ADD TABLE gencols(a, gen1);
+-- remove gencols from column list, when 'publish_generated_columns'=false
+ALTER PUBLICATION pub2 SET TABLE gencols(a);
+
+-- Add gencols in column list, when 'publish_generated_columns'=false.
+ALTER PUBLICATION pub2 SET TABLE gencols(a, gen1);
 
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
-DROP PUBLICATION pub3;
 DROP TABLE gencols;
 
 RESET client_min_messages;