On Thu, Jul 22, 2021 at 9:12 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On July 22, 2021 1:30 AM vignesh C <vignesh21@gmail.com> wrote
> > > I think PubType in this case should be 't' instead of 'e'. Please have a look.
> >
> > Thanks for reporting this issue, this issue is fixed in the attached v13 patch.
> > I have changed relation name pg_publication_schema to pg_publication_sch
> > so that the names are in similar lines with pg_publication_rel relation and similar
> > changes were done for variable names too.
>
> Hi,
>
> Thanks for the new version patches.
> I had a few comments.
>
> 1)
> +
> + appendPQExpBuffer(query, "ALTER PUBLICATION %s ", fmtId(pubrinfo->pubname));
> + appendPQExpBuffer(query, "ADD SCHEMA %s;\n", fmtId(schemainfo->dobj.name));
> +
>
> It seems we can combine these two function call.
> like appendPQExpBuffer(query, "ALTER PUBLICATION %s ADD SCHEMA %s;\n",
> fmtId(pubrinfo->pubname),
> fmtId(schemainfo->dobj.name));
>
Modified.
> 2)
> + footers[0] = pstrdup("Publications:");
> +
>
> This word seems need to be translated.
> footers[0] = pstrdup(_("Publications:"));
Modified
> 3)
> I think it might be better to add a testcase to cover the issue
> reported before [1].
>
> [1] https://www.postgresql.org/message-id/CAJcOf-dsKOYKmdrU5nwWeFoHvhiACbmw_KU%3DJQMEeDp6WwijqA%40mail.gmail.com
Added
> 4)
> Personally, the new name pg_publication_sch is not very easy to understand.
> (Maybe it's because I am not a native english speaker. If others feel ok,
> please ignore this comment)
I have changed it to pg_publication_schema as earlier, as Greg also
felt the other name was better.
Thanks for the comments, the attached v14 patch has the fixes for the same.
Regards,
Vignesh
