Re: Pgoutput not capturing the generated columns

On Wed, 6 Nov 2024 at 10:53, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Nov 6, 2024 at 10:26 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Wed, Nov 6, 2024 at 3:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Nov 6, 2024 at 7:34 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > > >
> > > > Hi Vignesh,
> > > >
> > > > Here are my review comments for patch v49-0001.
> > > >
> > >
> > > I have a question on the display of this new parameter.
> > >
> > > postgres=# \dRp+
> > >                                       Publication pub_gen
> > >   Owner   | All tables | Inserts | Updates | Deletes | Truncates | Via
> > > root | Generated columns
> > > ----------+------------+---------+---------+---------+-----------+----------+-------------------
> > >  KapilaAm | f          | t       | t       | t       | t         | f        | t
> > > Tables:
> > >     "public.test_gen"
> > >
> > > The current theory for the display of the "Generated Columns" option
> > > could be that let's add new parameters at the end which sounds
> > > reasonable. The other way to look at it is how it would be easier for
> > > users to interpret. I think the value of the "Via root" option should
> > > be either after "All tables" or at the end as that is higher level
> > > table information than operations or column-level information. As
> > > currently, it is at the end, so "Generated Columns" should be added
> > > before.
> > >
> > > Thoughts?
> > >
> >
> > FWIW, I've always felt the CREATE PUBLICATION parameters
> > publish
> > publish_via_root
> > publish_generated_columns
> >
> > Should be documented (e.g. on CREATE PUBLICATION page) in alphabetical order:
> > publish
> > publish_generated_columns
> > publish_via_root
> >
> > ~
> >
> > Following on from that. IMO it will make sense for the describe
> > (\dRp+) columns for those parameters to be in the same order as the
> > parameters in the documentation. So the end result would be the same
> > order as what you are wanting, even though the reason might be
> > different.
> >
>
> Sounds reasonable to me.

Updated the documentation and describe output accordingly.

> I have made some minor comments and function name changes in the
> attached. Please include in the next version.

Thanks, I have included the changes.

The attached v50 version patch has the changes for the same.

Regards,
Vignesh