On July 22, 2021 1:30 AM vignesh C <vignes...@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));


2)
+                       footers[0] = pstrdup("Publications:");
+

This word seems need to be translated.
footers[0] = pstrdup(_("Publications:"));

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

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)

Best regards,
Houzj

Reply via email to