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
From ca4a2950ceb62709e5642eb2dbf05b08f4d2220d Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.f...@cn.fujitsu.com>
Date: Wed, 19 Mar 2025 15:10:26 +0800
Subject: [PATCH] Some fixes
---
src/bin/pg_basebackup/pg_createsubscriber.c | 26 +++++++--------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c
b/src/bin/pg_basebackup/pg_createsubscriber.c
index d0e637d041f..1d31a7ebae2 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -1742,8 +1742,7 @@ check_and_drop_publications(PGconn *conn, struct
LogicalRepInfo *dbinfo)
pg_log_error("could not obtain publication information:
%s",
PQresultErrorMessage(res));
PQclear(res);
- disconnect_database(conn, false);
- return;
+ disconnect_database(conn, true);
}
/* Drop each publication */
@@ -1752,7 +1751,6 @@ check_and_drop_publications(PGconn *conn, struct
LogicalRepInfo *dbinfo)
&dbinfo->made_publication);
PQclear(res);
- pg_log_info("dropped all publications in database \"%s\"",
dbinfo->dbname);
}
else
drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
@@ -2081,10 +2079,7 @@ main(int argc, char **argv)
if
(!simple_string_list_member(&opt.objecttypes_to_remove, optarg))
simple_string_list_append(&opt.objecttypes_to_remove, optarg);
else
- {
- pg_log_error("object type \"%s\" is
specified more than once for --remove", optarg);
- exit(1);
- }
+ pg_fatal("object type \"%s\" is
specified more than once for --remove", optarg);
break;
case 's':
opt.socket_dir = pg_strdup(optarg);
@@ -2251,18 +2246,15 @@ main(int argc, char **argv)
}
/* Verify the object types specified for removal from the subscriber */
- if (opt.objecttypes_to_remove.head != NULL)
+ for (SimpleStringListCell *cell = opt.objecttypes_to_remove.head; cell;
cell = cell->next)
{
- for (SimpleStringListCell *cell =
opt.objecttypes_to_remove.head; cell; cell = cell->next)
+ if (pg_strcasecmp(cell->val, "publications") == 0)
+ dbinfos.objecttypes_to_remove |=
OBJECTTYPE_PUBLICATIONS;
+ else
{
- if (pg_strcasecmp(cell->val, "publications") == 0)
- dbinfos.objecttypes_to_remove |=
OBJECTTYPE_PUBLICATIONS;
- else
- {
- pg_log_error("invalid object type \"%s\"
specified for --remove", cell->val);
- pg_log_error_hint("The valid option is:
\"publications\"");
- exit(1);
- }
+ pg_log_error("invalid object type \"%s\" specified for
--remove", cell->val);
+ pg_log_error_hint("The valid option is:
\"publications\"");
+ exit(1);
}
}
--
2.30.0.windows.2
