Hi,
On 2014-05-25 22:35:24 +0900, Michael Paquier wrote:
> As written in subject, pg_recvlogical does not work properly with
> option -I but it should:
> $ pg_recvlogical -I 0/0
> pg_recvlogical: invalid option -- I
> Try "pg_recvlogical --help" for more information.
> $ pg_recvlogical --help | grep "\-I"
> -I, --startpos=PTR where in an existing slot should the streaming start
Good catch. Apparently only the long argument version ever worked...
> Attached patch corrects that, reshuffling at the same time the option
> letters parsed with getopt_long in alphabetical order.
Hm. Not a big fan of this in isolation. In the attached patch I've
reordered the options to all be ordered alphabetically, but only inside
the section they are in --help.
What do you think?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 85f54dafd13238ff831d1c66cddeaf071ad60708 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sun, 25 May 2014 18:47:05 +0200
Subject: [PATCH] Fix pg_recvlogical to accept the documented -I instead only
--startpos.
The bug was caused by omitting 'I:' from the short argument list to
getopt_long(). To make similar bugs in the future less likely reorder
options in --help, long and short option lists to be in the same,
alphabetical within groups, order.
Report and fix by Michael Paquier, some additional reordering by me.
---
src/bin/pg_basebackup/pg_recvlogical.c | 50 +++++++++++++++++-----------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 651cc40..cbbba9e 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -68,6 +68,8 @@ usage(void)
printf(_(" %s [OPTION]...\n"), progname);
printf(_("\nOptions:\n"));
printf(_(" -f, --file=FILE receive log into this file. - for stdout\n"));
+ printf(_(" -F --fsync-interval=SECS\n"
+ " frequency of syncs to the output file (default: %d)\n"), (fsync_interval / 1000));
printf(_(" -n, --no-loop do not loop on connection lost\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -80,8 +82,7 @@ usage(void)
printf(_(" -w, --no-password never prompt for password\n"));
printf(_(" -W, --password force password prompt (should happen automatically)\n"));
printf(_("\nReplication options:\n"));
- printf(_(" -F --fsync-interval=SECS\n"
- " frequency of syncs to the output file (default: %d)\n"), (fsync_interval / 1000));
+ printf(_(" -I, --startpos=PTR where in an existing slot should the streaming start\n"));
printf(_(" -o, --option=NAME[=VALUE]\n"
" specify option NAME with optional value VALUE, to be passed\n"
" to the output plugin\n"));
@@ -89,7 +90,6 @@ usage(void)
printf(_(" -s, --status-interval=SECS\n"
" time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
printf(_(" -S, --slot=SLOT use existing replication slot SLOT instead of starting a new one\n"));
- printf(_(" -I, --startpos=PTR where in an existing slot should the streaming start\n"));
printf(_("\nAction to be performed:\n"));
printf(_(" --create create a new replication slot (for the slotname see --slot)\n"));
printf(_(" --start start streaming in a replication slot (for the slotname see --slot)\n"));
@@ -600,6 +600,7 @@ main(int argc, char **argv)
static struct option long_options[] = {
/* general options */
{"file", required_argument, NULL, 'f'},
+ {"fsync-interval", required_argument, NULL, 'F'},
{"no-loop", no_argument, NULL, 'n'},
{"verbose", no_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
@@ -612,12 +613,11 @@ main(int argc, char **argv)
{"no-password", no_argument, NULL, 'w'},
{"password", no_argument, NULL, 'W'},
/* replication options */
+ {"startpos", required_argument, NULL, 'I'},
{"option", required_argument, NULL, 'o'},
{"plugin", required_argument, NULL, 'P'},
{"status-interval", required_argument, NULL, 's'},
- {"fsync-interval", required_argument, NULL, 'F'},
{"slot", required_argument, NULL, 'S'},
- {"startpos", required_argument, NULL, 'I'},
/* action */
{"create", no_argument, NULL, 1},
{"start", no_argument, NULL, 2},
@@ -647,7 +647,7 @@ main(int argc, char **argv)
}
}
- while ((c = getopt_long(argc, argv, "f:F:nvd:h:o:p:U:wWP:s:S:",
+ while ((c = getopt_long(argc, argv, "f:F:nvd:h:p:U:wWI:o:P:s:S:",
long_options, &option_index)) != -1)
{
switch (c)
@@ -656,6 +656,15 @@ main(int argc, char **argv)
case 'f':
outfile = pg_strdup(optarg);
break;
+ case 'F':
+ fsync_interval = atoi(optarg) * 1000;
+ if (fsync_interval < 0)
+ {
+ fprintf(stderr, _("%s: invalid fsync interval \"%s\"\n"),
+ progname, optarg);
+ exit(1);
+ }
+ break;
case 'n':
noloop = 1;
break;
@@ -688,6 +697,16 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
/* replication options */
+ case 'I':
+ if (sscanf(optarg, "%X/%X", &hi, &lo) != 2)
+ {
+ fprintf(stderr,
+ _("%s: could not parse start position \"%s\"\n"),
+ progname, optarg);
+ exit(1);
+ }
+ startpos = ((uint64) hi) << 32 | lo;
+ break;
case 'o':
{
char *data = pg_strdup(optarg);
@@ -720,28 +739,9 @@ main(int argc, char **argv)
exit(1);
}
break;
- case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < 0)
- {
- fprintf(stderr, _("%s: invalid fsync interval \"%s\"\n"),
- progname, optarg);
- exit(1);
- }
- break;
case 'S':
replication_slot = pg_strdup(optarg);
break;
- case 'I':
- if (sscanf(optarg, "%X/%X", &hi, &lo) != 2)
- {
- fprintf(stderr,
- _("%s: could not parse start position \"%s\"\n"),
- progname, optarg);
- exit(1);
- }
- startpos = ((uint64) hi) << 32 | lo;
- break;
/* action */
case 1:
do_create_slot = true;
--
1.8.3.251.g1462b67
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
