Re: proposal: pg_restore --convert-to-text

Daniel Verite <daniel@manitou-mail.org>

From: "Daniel Verite" <daniel@manitou-mail.org>
To: "Alvaro Herrera" <alvherre@2ndquadrant.com>
Cc: "José Arthur Benetasso Villanova" <jose.arthur@gmail.com>,"Imai, Yoshikazu" <imai.yoshikazu@jp.fujitsu.com>,"Euler Taveira" <euler@timbira.com.br>,"Tom Lane" <tgl@sss.pgh.pa.us>,"Andreas Karlsson" <andreas@proxel.se>,"Andrew Gierth" <andrew@tao11.riddles.org.uk>,"pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2019-07-03T16:43:46Z
Lists: pgsql-hackers
	Alvaro Herrera wrote:

> So you suggest that it should be 
> 
> pg_restore: error: one of -d/--dbname, -f/--file and -l/--list must be
> specified
> ?

I'd suggest this minimal fix :

(int argc, char **argv)
	/* Complain if neither -f nor -d was specified (except if dumping
TOC) */
	if (!opts->dbname && !opts->filename && !opts->tocSummary)
	{
-		pg_log_error("one of -d/--dbname and -f/--file must be
specified");
+		pg_log_error("-d/--dbname or -f/--file or -l/--list must be
specified");
+		fprintf(stderr, _("Try \"%s --help\" for more
information.\n"),
+				progname);
		exit_nicely(1);
	}

> So you suggest that it should be 
>   pg_restore [connection-option...] { -d | -f | -l } [option...] [filename]
> ?

Looking at the other commands, it doesn't seem that we use this form for
any of those that require at least one argument, for instance:

===
$ ./pg_basebackup 
pg_basebackup: error: no target directory specified
Try "pg_basebackup --help" for more information.

$ ./pg_basebackup --help
pg_basebackup takes a base backup of a running PostgreSQL server.

Usage:
  pg_basebackup [OPTION]...


$ ./pg_checksums 
pg_checksums: error: no data directory specified
Try "pg_checksums --help" for more information.

$ ./pg_checksums --help
pg_checksums enables, disables, or verifies data checksums in a PostgreSQL
database cluster.

Usage:
  pg_checksums [OPTION]... [DATADIR]


$ ./pg_rewind 
pg_rewind: error: no source specified (--source-pgdata or --source-server)
Try "pg_rewind --help" for more information.

$ ./pg_rewind  --help
pg_rewind resynchronizes a PostgreSQL cluster with another copy of the
cluster.

Usage:
  pg_rewind [OPTION]...
===

"pg_restore [OPTION]... [FILE]" appears to be consistent with those, even
with the new condition that no option is an error, so it's probably okay.

> Output target options:
>  -l, --list               print summarized TOC of the archive
>  -d, --dbname=NAME        connect to database name
>  -f, --file=FILENAME      output file name (- for stdout)

That makes sense. I would put this section first, before
"General options".


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite



Commits

  1. pg_restore: Require "-f -" to mean stdout