Adding -E switch to pg_dumpall

Started by Michael Paquierover 8 years ago6 messages
#1Michael Paquier
michael.paquier@gmail.com

Hi all,

While looking at a user problem, I got surprised that pg_dumpall does
not have a -E switch. This has been discussed a bit in the past like
here:
/messages/by-id/75E4C42D37E6A74E9FB57C2E9F1300D60107073E@tiger.nexperience.com

Now it is possible to enforce the encoding used by using
PGCLIENTENCODING but I think that a switch would be useful as well,
particularly for Windows where "set" needs to be used. Are there any
objections to a patch adding support for that? Such a patch should do:
- Call PQsetClientEncoding if an encoding is defined after getting a connection.
- Pass down -E to pg_dump calls if something is set.

Thoughts?
--
Michael

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

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#1)
1 attachment(s)
Re: Adding -E switch to pg_dumpall

On Fri, Jul 14, 2017 at 12:39 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

While looking at a user problem, I got surprised that pg_dumpall does
not have a -E switch. This has been discussed a bit in the past like
here:
/messages/by-id/75E4C42D37E6A74E9FB57C2E9F1300D60107073E@tiger.nexperience.com

Now it is possible to enforce the encoding used by using
PGCLIENTENCODING but I think that a switch would be useful as well,
particularly for Windows where "set" needs to be used. Are there any
objections to a patch adding support for that? Such a patch should do:
- Call PQsetClientEncoding if an encoding is defined after getting a connection.
- Pass down -E to pg_dump calls if something is set.

Thoughts?

Attached is a patch to add support for this switch. I am parking that
in the next CF.
--
Michael

Attachments:

pgdumpall-encoding-v1.patchapplication/octet-stream; name=pgdumpall-encoding-v1.patchDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index aa944a2e92..71075985af 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -100,6 +100,19 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>-E <replaceable class="parameter">encoding</replaceable></option></term>
+      <term><option>--encoding=<replaceable class="parameter">encoding</replaceable></option></term>
+      <listitem>
+       <para>
+        Create the dump in the specified character set encoding. By default,
+        the dump is created in the database encoding.  (Another way to get the
+        same result is to set the <envar>PGCLIENTENCODING</envar> environment
+        variable to the desired dump encoding.)
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-f <replaceable class="parameter">filename</replaceable></option></term>
       <term><option>--file=<replaceable class="parameter">filename</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index b14bb8e963..06a426568b 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -96,6 +96,7 @@ main(int argc, char *argv[])
 	static struct option long_options[] = {
 		{"data-only", no_argument, NULL, 'a'},
 		{"clean", no_argument, NULL, 'c'},
+		{"encoding", required_argument, NULL, 'E'},
 		{"file", required_argument, NULL, 'f'},
 		{"globals-only", no_argument, NULL, 'g'},
 		{"host", required_argument, NULL, 'h'},
@@ -145,6 +146,7 @@ main(int argc, char *argv[])
 	char	   *pguser = NULL;
 	char	   *pgdb = NULL;
 	char	   *use_role = NULL;
+	const char *dumpencoding = NULL;
 	trivalue	prompt_password = TRI_DEFAULT;
 	bool		data_only = false;
 	bool		globals_only = false;
@@ -202,7 +204,7 @@ main(int argc, char *argv[])
 
 	pgdumpopts = createPQExpBuffer();
 
-	while ((c = getopt_long(argc, argv, "acd:f:gh:l:oOp:rsS:tU:vwWx", long_options, &optindex)) != -1)
+	while ((c = getopt_long(argc, argv, "acd:E:f:gh:l:oOp:rsS:tU:vwWx", long_options, &optindex)) != -1)
 	{
 		switch (c)
 		{
@@ -219,6 +221,12 @@ main(int argc, char *argv[])
 				connstr = pg_strdup(optarg);
 				break;
 
+			case 'E':
+				dumpencoding = pg_strdup(optarg);
+				appendPQExpBufferStr(pgdumpopts, " -E ");
+				appendShellString(pgdumpopts, optarg);
+				break;
+
 			case 'f':
 				filename = pg_strdup(optarg);
 				appendPQExpBufferStr(pgdumpopts, " -f ");
@@ -450,6 +458,19 @@ main(int argc, char *argv[])
 		OPF = stdout;
 
 	/*
+	 * Set the client encoding if requested.
+	 */
+	if (dumpencoding)
+	{
+		if (PQsetClientEncoding(conn, dumpencoding) < 0)
+		{
+			fprintf(stderr, _("%s: invalid client encoding \"%s\" specified\n"),
+					progname, dumpencoding);
+			exit_nicely(1);
+		}
+	}
+
+	/*
 	 * Get the active encoding and the standard_conforming_strings setting, so
 	 * we know how to escape strings.
 	 */
@@ -584,6 +605,7 @@ help(void)
 	printf(_("\nOptions controlling the output content:\n"));
 	printf(_("  -a, --data-only              dump only the data, not the schema\n"));
 	printf(_("  -c, --clean                  clean (drop) databases before recreating\n"));
+	printf(_("  -E, --encoding=ENCODING      dump the data in encoding ENCODING\n"));
 	printf(_("  -g, --globals-only           dump only global objects, no databases\n"));
 	printf(_("  -o, --oids                   include OIDs in dump\n"));
 	printf(_("  -O, --no-owner               skip restoration of object ownership\n"));
#3Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Michael Paquier (#2)
Re: Adding -E switch to pg_dumpall

Hello Michaël-san,

Attached is a patch to add support for this switch. I am parking that
in the next CF.

I'm in favor of this feature for consistency with pg_dump, and as the
environment variable workaround is not specially elegant and can induce
issues of its own.

Patch applies and compiles.

No special regression tests are provided, but this seems ok to me as it
just forward the option to pg_dump, which I have no doubt is heavily
tested. Or not. Anyway it is consistent...

Manually tested with UTF8, latin1 (including conversion errors), non
existing.

Code is straightforward. Doc and help are both fine.

Ok for me. I switched the status to "Ready for committers".

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

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Fabien COELHO (#3)
Re: Adding -E switch to pg_dumpall

On Thu, Jul 20, 2017 at 11:17 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Ok for me. I switched the status to "Ready for committers".

Thanks for the review, Fabien.
--
Michael

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#4)
Re: Adding -E switch to pg_dumpall

On Fri, Jul 21, 2017 at 9:21 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Jul 20, 2017 at 11:17 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Ok for me. I switched the status to "Ready for committers".

Thanks for the review, Fabien.

LGTM. Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

#6Michael Paquier
michael.paquier@gmail.com
In reply to: Robert Haas (#5)
Re: Adding -E switch to pg_dumpall

On Sat, Sep 2, 2017 at 1:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jul 21, 2017 at 9:21 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Jul 20, 2017 at 11:17 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Ok for me. I switched the status to "Ready for committers".

Thanks for the review, Fabien.

LGTM. Committed.

Thanks for the commit.
--
Michael

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