RE: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.

From: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Shubham Khanna <khannashubham1197(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Date: 2025-03-19 07:14:14
Message-ID: OS3PR01MB57181D8F376A07BAD97A404294D92@OS3PR01MB5718.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 19, 2025 at 2:55 PM Shubham Khanna wrote:
>
> I have merged the changes and prepared the latest patch. The attached
> patch contains the suggested changes.

Thanks for updating the patch. Here are few comments:

1.
pg_log_error("object type \"%s\" is specified more than once for --remove", optarg);
exit(1);

Consider using pg_fatal for simplicity.

2.

+ /* Fetch all publication names */
+ res = PQexec(conn, "SELECT pubname FROM pg_catalog.pg_publication;");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ pg_log_error("could not obtain publication information: %s",
+ PQresultErrorMessage(res));
+ PQclear(res);
+ disconnect_database(conn, false);
+ return;
+ }

I think we should exit here for consistency, as performed in similar cases.

3.

+ pg_log_info("dropped all publications in database \"%s\"", dbinfo->dbname);

This message may be misleading if some publications were not dropped
successfully, as drop_publication does not exit on a drop failure.

4.

if (opt.remove_objects.head != NULL)
{
for (SimpleStringListCell *cell = opt.remove_objects.head; cell; cell = cell->next)
{

I think the first null test is redundant.

I have attached a patch with the proposed changes. If you agree with these
modifications, please merge them.

Best Regards,
Hou zj

Attachment Content-Type Size
0001-Some-fixes.patch.txt text/plain 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2025-03-19 07:17:23 Re: Snapshot related assert failure on skink
Previous Message Gabriele Bartolini 2025-03-19 07:03:08 Re: RFC: Additional Directory for Extensions