Horton
Menu

Re: Pgoutput not capturing the generated columns

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Shubham Khanna <khannashubham1197@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>, pgsql-hackers@lists.postgresql.org, euler@eulerto.com
Date: 2024-10-25T10:24:05Z
Lists: pgsql-hackers 
On Fri, Oct 25, 2024 at 12:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Oct 24, 2024 at 8:50 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > The v42 version patch attached at [1] has the changes for the same.
> >
>
> Some more comments:
>

1.
+pgoutput_pubgencol_init(PGOutputData *data, List *publications,
+ RelationSyncEntry *entry)

Can we name it as check_and_init_gencol? I don't know if it is a good
idea to append a prefix pgoutput for local functions. It is primarily
used for exposed functions from pgoutput.c. I see that in a few cases
we do that for local functions as well but that is not a norm.

A related point:
+ /* Initialize publish generated columns value */
+ pgoutput_pubgencol_init(data, rel_publications, entry);

Accordingly change this comment to something like: "Check whether to
publish to generated columns.".

2.
+/*
+ * Returns true if the relation has column list associated with the
+ * publication, false if the relation has no column list associated with the
+ * publication.
+ */
+bool
+is_column_list_publication(Publication *pub, Oid relid)
...
...

How about naming the above function as has_column_list_defined()?
Also, you can write the above comment as: "Returns true if the
relation has column list associated with the publication, false
otherwise."

3.
+ /*
+ * The column list takes precedence over pubgencols, so skip checking
+ * column list publications.
+ */
+ if (is_column_list_publication(pub, entry->publish_as_relid))

Let's change this comment to: "The column list takes precedence over
publish_generated_columns option. Those will be checked later, see
pgoutput_column_list_init."

-- 
With Regards,
Amit Kapila.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Generated column replication.

  2. Rename pubgencols_type to pubgencols in pg_publication.

  3. Doc: Fix column name in pg_publication catalog.

  4. Fix buildfarm failure introduced by commit e65dbc9927.

  5. Change publication's publish_generated_columns option type to enum.

  6. Fix \dRp+ output when describing publications with a lower server version.

  7. Replicate generated columns when 'publish_generated_columns' is set.

  8. Doc: Update the behavior of generated columns in Logical Replication.

  9. Replicate generated columns when specified in the column list.

  10. Doc: Generated columns are skipped for logical replication.