PS_NITPICKS_GENCOLS_V10001.txt

text/plain

Filename: PS_NITPICKS_GENCOLS_V10001.txt
Type: text/plain
Part: 0
Message: Re: Pgoutput not capturing the generated columns
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index a662a45..0d76674 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -241,22 +241,17 @@ has_column_list_defined(Publication *pub, Oid relid)
 	cftuple = SearchSysCache2(PUBLICATIONRELMAP,
 							  ObjectIdGetDatum(relid),
 							  ObjectIdGetDatum(pub->oid));
-	if (HeapTupleIsValid(cftuple))
-	{
-		/* Lookup the column list attribute. */
-		(void) SysCacheGetAttr(PUBLICATIONRELMAP, cftuple,
-							   Anum_pg_publication_rel_prattrs,
-							   &isnull);
-		if (!isnull)
-		{
-			ReleaseSysCache(cftuple);
-			return true;
-		}
+	if (!HeapTupleIsValid(cftuple))
+		return false;
 
-		ReleaseSysCache(cftuple);
-	}
+	/* Lookup the column list attribute. */
+	(void) SysCacheGetAttr(PUBLICATIONRELMAP, cftuple,
+						   Anum_pg_publication_rel_prattrs,
+						   &isnull);
+	ReleaseSysCache(cftuple);
 
-	return false;
+	/* Was a column list found for this relation? */
+	return isnull ? false : true;
 }
 
 /*
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index 62b79cf..e928d94 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -1251,30 +1251,34 @@ logicalrep_message_type(LogicalRepMsgType action)
 /*
  * Check if the column 'att' of a table should be published.
  *
- * 'columns' represents the column list specified for that table in the
- * publication.
+ * 'columns' represents the publication column list (if any) for that table.
  *
- * Note that generated columns can be present only in 'columns' list.
+ * Note that generated columns can be published only when present in a
+ * publication column list, or (if there is no column list), when the
+ * publication parameter 'publish_generated_columns' it true.
  */
 bool
 logicalrep_should_publish_column(Form_pg_attribute att, Bitmapset *columns,
-								 bool pubgencols)
+								 bool pubgencols_option)
 {
 	if (att->attisdropped)
 		return false;
 
-	/*
-	 * Skip publishing generated columns if they are not included in the
-	 * column list or if the option is not specified.
-	 */
-	if (!columns && !pubgencols && att->attgenerated)
-		return false;
-
-	/*
-	 * Check if a column is covered by a column list.
-	 */
-	if (columns && !bms_is_member(att->attnum, columns))
-		return false;
-
-	return true;
+	if (columns)
+	{
+		/*
+		 * Has a column list:
+		 * Publish only cols named in that list.
+		 */
+		return bms_is_member(att->attnum, columns);
+	}
+	else
+	{
+		/*
+		 * Has no column list:
+		 * Publish generated cols only if 'publish_generated_cols' is true.
+		 * Publish all non-generated cols.
+		 */
+	return att->attgenerated ? pubgencols_option : true;
+	}
 }
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index d94e120..6da57e2 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1022,7 +1022,7 @@ check_and_init_gencol(PGOutputData *data, List *publications,
 	ListCell   *lc;
 	bool		first = true;
 
-	/* Check if there is any generated column present */
+	/* Check if there is any generated column present. */
 	for (int i = 0; i < desc->natts; i++)
 	{
 		Form_pg_attribute att = TupleDescAttr(desc, i);
@@ -1034,7 +1034,7 @@ check_and_init_gencol(PGOutputData *data, List *publications,
 		}
 	}
 
-	/* There is no generated columns to be published */
+	/* There are no generated columns to be published. */
 	if (!gencolpresent)
 	{
 		entry->pubgencols = false;
@@ -1080,7 +1080,7 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 	ListCell   *lc;
 	bool		first = true;
 	Relation	relation = RelationIdGetRelation(entry->publish_as_relid);
-	bool		collistpubexist = false;
+	bool		found_pub_with_collist = false;
 	Bitmapset  *relcols = NULL;
 
 	/*
@@ -1111,8 +1111,6 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 		 */
 		if (!pub->alltables)
 		{
-			bool		pub_no_list = true;
-
 			/*
 			 * Check for the presence of a column list in this publication.
 			 *
@@ -1126,17 +1124,19 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 
 			if (HeapTupleIsValid(cftuple))
 			{
+				bool isnull;
+
 				/* Lookup the column list attribute. */
 				cfdatum = SysCacheGetAttr(PUBLICATIONRELMAP, cftuple,
 										  Anum_pg_publication_rel_prattrs,
-										  &pub_no_list);
+										  &isnull);
 
 				/* Build the column list bitmap in the per-entry context. */
-				if (!pub_no_list)	/* when not null */
+				if (!isnull)	/* when not null */
 				{
 					pgoutput_ensure_entry_cxt(data, entry);
 
-					collistpubexist = true;
+					found_pub_with_collist = true;
 					cols = pub_collist_to_bitmapset(cols, cfdatum,
 													entry->entry_cxt);
 				}
@@ -1146,10 +1146,10 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 		}
 
 		/*
-		 * For non-column list publications—such as TABLE (without a column
-		 * list), ALL TABLES, or ALL TABLES IN SCHEMA publications consider
-		 * all columns of the table, including generated columns, based on the
-		 * pubgencols option.
+		 * For non-column list publications — e.g. TABLE (without a column list),
+		 * ALL TABLES, or ALL TABLES IN SCHEMA, we consider all columns of the
+		 * table (including generated columns if 'publish_generated_columns'
+		 * option is true).
 		 */
 		if (!cols)
 		{
@@ -1186,7 +1186,7 @@ pgoutput_column_list_init(PGOutputData *data, List *publications,
 	 * If no column list publications exit, columns will be selected later
 	 * according to the generated columns option.
 	 */
-	if (!collistpubexist)
+	if (!found_pub_with_collist)
 		entry->columns = NULL;
 
 	RelationClose(relation);