On Tue, Nov 5, 2024 at 7:00 AM Peter Smith <smithpb2...@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.
