Re: Pgoutput not capturing the generated columns

On Tue, Nov 5, 2024 at 12:32 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> has_column_list_defined:
>
> 1.
> + if (HeapTupleIsValid(cftuple))
> + {
> + bool isnull = true;
> +
> + /* Lookup the column list attribute. */
> + (void) SysCacheGetAttr(PUBLICATIONRELMAP, cftuple,
> +    Anum_pg_publication_rel_prattrs,
> +    &isnull);
>
> AFAIK it is not necessary to assign a default value to 'isnull' here.
> e.g. most of the other 100s of calls to SysCacheGetAttr elsewhere in
> PostgreSQL source don't bother to do this.
>

Can we try to reuse this new function has_column_list_defined() in
pgoutput_column_list_init() where we are trying to check and form a
column list? For that, we need to change this function such that it
also returns a column list when requested.

Some more comments:
1.
 extern bool is_schema_publication(Oid pubid);
+extern bool has_column_list_defined(Publication *pub, Oid relid);

The order of declaration doesn't match with its definition in .c file.
It would be better to define this function after
is_schema_publication().

2.
+ * 'include_gencols' flag indicates whether generated columns should be
+ * published when there is no column list. Typically, this will have the same
+ * value as the 'publish_generated_columns' publication parameter.
+ *
+ * Note that generated columns can be published only when present in a
+ * publication column list, or when include_gencols is true.
  */
 bool
-logicalrep_should_publish_column(Form_pg_attribute att, Bitmapset *columns)
+logicalrep_should_publish_column(Form_pg_attribute att, Bitmapset *columns,
+ bool include_gencols)
 {
  if (att->attisdropped)
  return false;

- /*
- * Skip publishing generated columns if they are not included in the
- * column list.
- */
- if (!columns && att->attgenerated)
- return false;
+ /* If a column list is provided, publish only the cols in that list. */
+ if (columns)
+ return bms_is_member(att->attnum, columns);

  /*
- * Check if a column is covered by a column list.
+ * If no column list is provided, generated columns will be published only
+ * if include_gencols is true, while all non-generated columns will always
+ * be published.

The similar information is repeated multiple times in different words.
I have adjusted the comments in this part of the code to avoid
repetition.

I have changed comments at a few other places in the patch. See attached.

-- 
With Regards,
Amit Kapila.