pg_rewind: Should abort if both --source-pgdata and --source-server are specified

Started by Michael Banckover 9 years ago2 messages
#1Michael Banck
michael.banck@credativ.de
1 attachment(s)

Hi,

ISTM that pg_rewind's --source-pgdata and --source-server options are
mutually exclusive, and the synopsis in the documentation seems to
indicate that as well:

|pg_rewind [option...] {-D | --target-pgdata} directory
|{--source-pgdata=directory | --source-server=connstr}

However, there is no such check in the code.

I've seen people assume --source-pgdata is supposed to be the data
directory location on the remote server if they specify --source-server
as well, and are then confused by error messages.

So I think pg_rewind should abort in this case. Patch for that attached.

Thoughts?

Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.banck@credativ.de

credativ GmbH, HRB M�nchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 M�nchengladbach
Gesch�ftsf�hrung: Dr. Michael Meskes, J�rg Folz, Sascha Heuer

Attachments:

pg_rewind.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 5fdd4c5..dd62dd0 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -162,6 +162,13 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	if (datadir_source != NULL && connstr_source != NULL)
+	{
+		fprintf(stderr, _("%s: only one of --source-pgdata or --source-server can be specified\n"), progname);
+		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		exit(1);
+	}
+
 	if (datadir_target == NULL)
 	{
 		fprintf(stderr, _("%s: no target data directory specified (--target-pgdata)\n"), progname);
#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Banck (#1)
Re: pg_rewind: Should abort if both --source-pgdata and --source-server are specified

On 10/07/2016 01:34 PM, Michael Banck wrote:

Hi,

ISTM that pg_rewind's --source-pgdata and --source-server options are
mutually exclusive, and the synopsis in the documentation seems to
indicate that as well:

|pg_rewind [option...] {-D | --target-pgdata} directory
|{--source-pgdata=directory | --source-server=connstr}

However, there is no such check in the code.

I've seen people assume --source-pgdata is supposed to be the data
directory location on the remote server if they specify --source-server
as well, and are then confused by error messages.

So I think pg_rewind should abort in this case. Patch for that attached.

Agreed. Committed, thanks!

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers