Re: Pgoutput not capturing the generated columns

On Thu, Oct 24, 2024 at 8:50 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> The v42 version patch attached at [1] has the changes for the same.
>

Some more comments:
1.
@@ -1017,7 +1089,31 @@ pgoutput_column_list_init(PGOutputData *data,
List *publications,
{
ListCell *lc;
bool first = true;
+ Bitmapset *relcols = NULL;
Relation relation = RelationIdGetRelation(entry->publish_as_relid);
+ TupleDesc desc = RelationGetDescr(relation);
+ MemoryContext oldcxt = NULL;
+ bool collistpubexist = false;
+
+ pgoutput_ensure_entry_cxt(data, entry);
+
+ oldcxt = MemoryContextSwitchTo(entry->entry_cxt);
+
+ /*
+ * Prepare the columns that will be published for FOR ALL TABLES and
+ * FOR TABLES IN SCHEMA publication.
+ */
+ for (int i = 0; i < desc->natts; i++)
+ {
+ Form_pg_attribute att = TupleDescAttr(desc, i);
+
+ if (att->attisdropped || (att->attgenerated && !entry->pubgencols))
+ continue;
+
+ relcols = bms_add_member(relcols, att->attnum);
+ }
+
+ MemoryContextSwitchTo(oldcxt);

This code is unnecessary for cases when the table's publication has a
column list. So, I suggest to form this list only when required. Also,
have an assertion that pubgencols value for entry and publication
matches.

2.
@@ -1115,10 +1186,17 @@ pgoutput_column_list_init(PGOutputData *data,
List *publications,
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot use different column lists for table \"%s.%s\" in
different publications",
- get_namespace_name(RelationGetNamespace(relation)),
- RelationGetRelationName(relation)));
+ get_namespace_name(RelationGetNamespace(relation)),
+ RelationGetRelationName(relation)));

Is there a reason to make the above change? It appears to be a spurious change.

3.
+ /* Check if there is any generated column present */
+ for (int i = 0; i < desc->natts; i++)
+ {
+ Form_pg_attribute att = TupleDescAttr(desc, i);
+ if (att->attgenerated)

Add one empty line between the above two lines.

4.
+ else if (entry->pubgencols != pub->pubgencols)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot use different values of publish_generated_columns for
table \"%s.%s\" in different publications",
+ get_namespace_name(RelationGetNamespace(relation)),
+ RelationGetRelationName(relation)));

The last two lines are not aligned.

--
With Regards,
Amit Kapila.