RE: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility. - Mailing list 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