On Tue, Nov 5, 2024 at 7:00 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
>
> ~
>
> 1b.
> Everywhere in this patch (except here), this is called the
> 'publish_generated_columns' parameter (not "option") so it should be
> called a parameter here also. Anyway, apparently that is the docs rule
> -- see [1].
>
In the thread you linked, we have decided to name 'failover' an
option. I feel the same should be followed here but I agree that we
should spell it consistently throughout the patch.
>
> ======
> doc/src/sgml/protocol.sgml
>
> 2.
> - Next, one of the following submessages appears for each column:
> + Next, one of the following submessages appears for each published column:
>
> The change is OK. But, note that there are other descriptions just
> like this one on the same page, so if you are going to say "published"
> here, then to be consistent you probably want to consider updating the
> other places as well.
>
Are you referring to the existing message: "Next, the following
message part appears for each column included in the publication:"? If
so, we can change it to make it the same but the current one also
looks okay. We can consider changing it separately if required after
this patch.
> ======
> src/backend/replication/pgoutput/pgoutput.c
>
> 6.
> + /*
> + * Include publishing generated columns if 'publish_generated_columns'
> + * parameter is set to true, this will be set only if the relation
> + * contains any generated column.
> + */
> + bool include_gencols;
> +
>
> Minor rewording.
>
> SUGGESTION:
> Include generated columns for publication is set true if
>
/set true/set to true
>
> ======
> [1] option versus parameter -
> https://www.postgresql.org/message-id/CAKFQuwZVJ%2B_Z0pMX%3DBBKF9A6skVqiv89gxEgFOX7cwtWJj-Ccw%40mail.gmail.com
>
--
With Regards,
Amit Kapila.
