RE: Added schema level support for publication.

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>
Subject: RE: Added schema level support for publication.
Date: 2021-07-22 03:42:10
Message-ID: OS0PR01MB5716A0E509064D01495E240A94E49@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On July 22, 2021 1:30 AM vignesh C <vignesh21(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-07-22 03:55:47 Re: [bug?] Missed parallel safety checks, and wrong parallel safety
Previous Message David Rowley 2021-07-22 02:04:04 Re: [PATCH] Use optimized single-datum tuplesort in ExecSort