Re: Pgoutput not capturing the generated columns

On Fri, 17 Jan 2025 at 11:23, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Vignesh.
>
> Some review comments for patch v52-0003
>
> ======
>
> 1. GENERAL - change to use enum.
>
> On Thu, Jan 16, 2025 at 7:47 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Wed, 15 Jan 2025 at 11:17, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > > 2.
> > > As suggested in more detail below, I think it would be better if you
> > > can define a C code enum type for these potential values instead of
> > > just using #define macros and char. I guess that will impact a lot of
> > > the APIs.
> >
> > If we change it to enum, we will not be able to access
> > PUBLISH_GENCOLS_NONE and PUBLISH_GENCOLS_STORED from describe.c files.
> > Maybe that is the reason the macros were used in the case of
> > pg_subscription.h also.
>
> Hm. I am not sure. Can't you just define the enum inside the #ifdef
> EXPOSE_TO_CLIENT_CODE? I saw some examples of this already (see
> src/include/catalog/pg_cast.h)
>
> e.g. I tried following, which compiles for me:
>
> #ifdef EXPOSE_TO_CLIENT_CODE
>
> typedef enum PublishGencolsType
> {
> /* Generated columns present should not be replicated. */
> PUBLISH_GENCOLS_NONE = 'n',
>
> /* Generated columns present should be replicated. */
> PUBLISH_GENCOLS_STORED = 's',
>
> } PublishGencolsType;
>
> #endif /* EXPOSE_TO_CLIENT_CODE */
>
> typedef struct Publication
> {
> Oid oid;
> char *name;
> bool alltables;
> bool pubviaroot;
> PublishGencolsType pubgencols_type;
> PublicationActions pubactions;
> } Publication;

Yes, the compilation seems fine but listPublications which uses
CppAsString2(PUBLISH_GENCOLS_NONE) does not get 'n' but gets it as
publish_gencols_none like below:
postgres=# \dRp
ERROR: column "publish_gencols_none" does not exist
LINE 9: WHEN PUBLISH_GENCOLS_NONE THEN 'non

The rest of the comments are handled and also the comments from [1]
are fixed in the v53 version patch attached.

[1] - https://www.postgresql.org/message-id/CAHut%2BPuykmXF57T9AcBnsVqQEddmpBeHZvnGdJt%3DbyYozXAcSg%40mail.gmail.com

Regards,
Vignesh