public inbox for [email protected]
help / color / mirror / Atom feedFrom: vignesh C <[email protected]>
To: Amit Kapila <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Improve error reporting for few options in pg_createsubscriber
Date: Fri, 4 Apr 2025 10:11:24 +0530
Message-ID: <CALDaNm26TqzK8jcOmMbFJruh7JHFnVcGAtNju3DCyj3844OFeA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1Jbq35CpKXpii=ORLVSaXoUbLNbTY031W=xKOncf3hR5Q@mail.gmail.com>
References: <CALDaNm0HxF1RH27LP7VisLzNsSJbssy8a64M5p6UduDaBq6-ag@mail.gmail.com>
<CAA4eK1Jbq35CpKXpii=ORLVSaXoUbLNbTY031W=xKOncf3hR5Q@mail.gmail.com>
On Fri, 4 Apr 2025 at 09:36, Amit Kapila <[email protected]> wrote:
>
> A few comments:
> *
> pg_createsubscriber.exe -D ..\..\data_standby -d db1 -d db2 -d db1
> --publication pub1 --publication pub1 -P "dbname=postgres port=5432"
> -p 5444 --dry-run
> pg_createsubscriber: error: database "db1" specified more than once
> for --database
>
> Even when user has used short form, we are displaying long form for
> database. I suggest to use both short and long form where applicable.
Fixed this.
> *
> "invalid object type \"%s\" specified for --remove". Isn't it better
> to use the short form as well in this error message?
Fixed this.
The attached v2 version patch has the changes for the same.
Regards,
Vignesh
Attachments:
[text/x-patch] v2-0001-Improve-error-reporting-consistency-in-pg_creates.patch (4.1K, ../CALDaNm26TqzK8jcOmMbFJruh7JHFnVcGAtNju3DCyj3844OFeA@mail.gmail.com/2-v2-0001-Improve-error-reporting-consistency-in-pg_creates.patch)
download | inline diff:
From 40ef017e6e5f22fb8650495935c186b5a4a56878 Mon Sep 17 00:00:00 2001
From: Vignesh <[email protected]>
Date: Sat, 22 Mar 2025 18:56:15 +0530
Subject: [PATCH v2] Improve error reporting consistency in pg_createsubscriber
Ensure consistent error reporting in pg_createsubscriber
by including the option name in error messages. Additionally,
replace pg_log_error and exit calls with pg_fatal.
---
src/bin/pg_basebackup/pg_createsubscriber.c | 26 +++++--------------
.../t/040_pg_createsubscriber.pl | 4 +--
2 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index eed3793c816..1d2f39e3926 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -2124,10 +2124,7 @@ main(int argc, char **argv)
num_dbs++;
}
else
- {
- pg_log_error("database \"%s\" specified more than once", optarg);
- exit(1);
- }
+ pg_fatal("database \"%s\" specified more than once for -d/--database", optarg);
break;
case 'D':
subscriber_dir = pg_strdup(optarg);
@@ -2146,7 +2143,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_fatal("object type \"%s\" is specified more than once for --remove", optarg);
+ pg_fatal("object type \"%s\" is specified more than once for -R/--remove", optarg);
break;
case 's':
opt.socket_dir = pg_strdup(optarg);
@@ -2174,10 +2171,7 @@ main(int argc, char **argv)
num_pubs++;
}
else
- {
- pg_log_error("publication \"%s\" specified more than once", optarg);
- exit(1);
- }
+ pg_fatal("publication \"%s\" specified more than once for --publication", optarg);
break;
case 3:
if (!simple_string_list_member(&opt.replslot_names, optarg))
@@ -2186,10 +2180,7 @@ main(int argc, char **argv)
num_replslots++;
}
else
- {
- pg_log_error("replication slot \"%s\" specified more than once", optarg);
- exit(1);
- }
+ pg_fatal("replication slot \"%s\" specified more than once for --replication-slot", optarg);
break;
case 4:
if (!simple_string_list_member(&opt.sub_names, optarg))
@@ -2198,10 +2189,7 @@ main(int argc, char **argv)
num_subs++;
}
else
- {
- pg_log_error("subscription \"%s\" specified more than once", optarg);
- exit(1);
- }
+ pg_fatal("subscription \"%s\" specified more than once for --subscription", optarg);
break;
default:
/* getopt_long already emitted a complaint */
@@ -2226,7 +2214,7 @@ main(int argc, char **argv)
if (bad_switch)
{
- pg_log_error("%s cannot be used with --all", bad_switch);
+ pg_log_error("%s cannot be used with -a/--all", bad_switch);
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
@@ -2352,7 +2340,7 @@ main(int argc, char **argv)
dbinfos.objecttypes_to_remove |= OBJECTTYPE_PUBLICATIONS;
else
{
- pg_log_error("invalid object type \"%s\" specified for --remove", cell->val);
+ pg_log_error("invalid object type \"%s\" specified for -R/--remove", cell->val);
pg_log_error_hint("The valid option is: \"publications\"");
exit(1);
}
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 80153f7d77e..2d532fee567 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -399,7 +399,7 @@ command_fails_like(
'--database' => $db1,
'--all',
],
- qr/--database cannot be used with --all/,
+ qr/--database cannot be used with -a\/--all/,
'fail if --database is used with --all');
# run pg_createsubscriber with '--publication' and '--all' and verify
@@ -416,7 +416,7 @@ command_fails_like(
'--all',
'--publication' => 'pub1',
],
- qr/--publication cannot be used with --all/,
+ qr/--publication cannot be used with -a\/--all/,
'fail if --publication is used with --all');
# run pg_createsubscriber with '--all' option
--
2.43.0
view thread (5+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Improve error reporting for few options in pg_createsubscriber
In-Reply-To: <CALDaNm26TqzK8jcOmMbFJruh7JHFnVcGAtNju3DCyj3844OFeA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox