ICU: PGPRO-1265: Option --add-collprovider for pg_dump[all]

This option is recommended when upgrading with pg_dump[all] and pg_restore from
previous versions of Postgres Pro as well as from all versions of PostgreSQL
(it's automatically used in case of a binary upgrade; and it actually does
nothing for the current version of Postgres Pro). If you do not want to manually
fix the scripts (or do not know how to do it correctly) this helps to avoid mess
when the templated database and the created database actually use different
collation providers that are ommitted. In this case the check constrains that
use the default collation of the database may change and the COPY command may
end with fail.

Author: Marina Polyakova

doc/src/sgml/ref/pg_dump.sgml

@@ -638,6 +638,27 @@ doc/src/sgml/ref/pg_dump.sgml
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--add-collprovider</option></term>
+      <listitem>
+       <para>
+        This option is recommended when upgrading with
+        <application>pg_dump</application> and
+        <application>pg_restore</application> from previous versions of
+        <productname>&project;</> as well as from all versions of
+        <productname>PostgreSQL</> (it's automatically used in case of a binary
+        upgrade; and it actually does nothing for the current version of
+        <productname>&project;</>). If you do not want to manually fix the
+        scripts (or do not know how to do it correctly) this helps to avoid mess
+        when the templated database and the created database actually use
+        different collation providers that are ommitted (see <xref
+        linkend="collation-managing">). In this case the check constrains that
+        use the default collation of the database may change and the
+        <command>COPY</command> command may end with fail.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--binary-upgrade</option></term>
       <listitem>

doc/src/sgml/ref/pg_dumpall.sgml

@@ -231,6 +231,27 @@ doc/src/sgml/ref/pg_dumpall.sgml
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--add-collprovider</option></term>
+      <listitem>
+       <para>
+        This option is recommended when upgrading with
+        <application>pg_dumpall</application> and
+        <application>pg_restore</application> from previous versions of
+        <productname>&project;</> as well as from all versions of
+        <productname>PostgreSQL</> (it's automatically used in case of a binary
+        upgrade; and it actually does nothing for the current version of
+        <productname>&project;</>). If you do not want to manually fix the
+        scripts (or do not know how to do it correctly) this helps to avoid mess
+        when the templated database and the created database actually use
+        different collation providers that are ommitted (see <xref
+        linkend="collation-managing">). In this case the check constrains that
+        use the default collation of the database may change and the
+        <command>COPY</command> command may end with fail.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--binary-upgrade</option></term>
       <listitem>

src/bin/pg_dump/pg_dump.c

@@ -135,6 +135,12 @@ char		g_comment_end[10];
 
 static const CatalogId nilCatalogId = {0, 0};
 
+/*
+ * Whether to add the collation provider to the non-empty LC_COLLATE settings in
+ * the database creation; always true for binary upgrade.
+ */
+static bool add_collprovider = false;
+
 /*
  * Macro for producing quoted, schema-qualified name of a dumpable object.
  * Note implicit dependence on "fout"; we should get rid of that argument.
@@ -372,6 +378,7 @@ main(int argc, char **argv)
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 7},
+		{"add-collprovider", no_argument, NULL, 8},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -556,6 +563,10 @@ main(int argc, char **argv)
 				dosync = false;
 				break;
 
+			case 8:
+				add_collprovider = true;
+				break;
+
 			default:
 				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 				exit_nicely(1);
@@ -583,14 +594,18 @@ main(int argc, char **argv)
 	if (dopt.column_inserts)
 		dopt.dump_inserts = 1;
 
-	/*
-	 * Binary upgrade mode implies dumping sequence data even in schema-only
-	 * mode.  This is not exposed as a separate option, but kept separate
-	 * internally for clarity.
-	 */
 	if (dopt.binary_upgrade)
+	{
+		/*
+		 * Binary upgrade mode implies dumping sequence data even in schema-only
+		 * mode.  This is not exposed as a separate option, but kept separate
+		 * internally for clarity.
+		 */
 		dopt.sequence_data = 1;
 
+		add_collprovider = true;
+	}
+
 	if (dopt.dataOnly && dopt.schemaOnly)
 	{
 		write_msg(NULL, "options -s/--schema-only and -a/--data-only cannot be used together\n");
@@ -959,6 +974,7 @@ help(const char *progname)
 	printf(_("  -t, --table=TABLE            dump the named table(s) only\n"));
 	printf(_("  -T, --exclude-table=TABLE    do NOT dump the named table(s)\n"));
 	printf(_("  -x, --no-privileges          do not dump privileges (grant/revoke)\n"));
+	printf(_("  --add-collprovider           add provider for database LC_COLLATE settings\n"));
 	printf(_("  --binary-upgrade             for use by upgrade utilities only\n"));
 	printf(_("  --column-inserts             dump data as INSERT commands with column names\n"));
 	printf(_("  --disable-dollar-quoting     disable dollar quoting, use SQL standard quoting\n"));
@@ -2573,7 +2589,7 @@ dumpDatabase(Archive *fout)
 		char *dbcollate;
 		appendPQExpBufferStr(creaQry, " LC_COLLATE = ");
 		dbcollate = pg_strdup(collate);
-		if (dopt->binary_upgrade)
+		if (add_collprovider)
 			dbcollate = addCollProvider(dbcollate,
 										fout->remoteVersion,
 										fout->remoteEdition,

src/bin/pg_dump/pg_dumpall.c

@@ -92,6 +92,8 @@ static char role_catalog[10];
 static FILE *OPF;
 static char *filename = NULL;
 
+static int	add_collprovider = 0;
+
 #define exit_nicely(code) exit(code)
 
 int
@@ -140,6 +142,7 @@ main(int argc, char *argv[])
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
+		{"add-collprovider", no_argument, &add_collprovider, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -364,6 +367,10 @@ main(int argc, char *argv[])
 		exit_nicely(1);
 	}
 
+	/* Always add the collation provider in the binary upgrade */
+	if (binary_upgrade)
+		add_collprovider++;
+
 	/*
 	 * If password values are not required in the dump, switch to using
 	 * pg_roles which is equally useful, just more likely to have unrestricted
@@ -399,6 +406,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
 	if (no_unlogged_table_data)
 		appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
+	if (add_collprovider)
+		appendPQExpBufferStr(pgdumpopts, " --add-collprovider");
 
 	/*
 	 * If there was a database specified on the command line, use that,
@@ -596,6 +605,7 @@ help(void)
 	printf(_("  -S, --superuser=NAME         superuser user name to use in the dump\n"));
 	printf(_("  -t, --tablespaces-only       dump only tablespaces, no databases or roles\n"));
 	printf(_("  -x, --no-privileges          do not dump privileges (grant/revoke)\n"));
+	printf(_("  --add-collprovider           add provider for database LC_COLLATE settings\n"));
 	printf(_("  --binary-upgrade             for use by upgrade utilities only\n"));
 	printf(_("  --column-inserts             dump data as INSERT commands with column names\n"));
 	printf(_("  --disable-dollar-quoting     disable dollar quoting, use SQL standard quoting\n"));
@@ -1461,18 +1471,21 @@ dumpCreateDB(PGconn *conn)
 				appendStringLiteralConn(buf, dbencoding, conn);
 			}
 
-			if (default_collate && strcmp(dbcollate, default_collate) != 0)
+			if (default_collate && strlen(dbcollate) > 0)
 			{
-				char *dbcollate_full_name;
-				appendPQExpBufferStr(buf, " LC_COLLATE = ");
-				dbcollate_full_name = pg_strdup(dbcollate);
-				if (binary_upgrade)
+				char *dbcollate_full_name = pg_strdup(dbcollate);
+
+				if (add_collprovider)
 					dbcollate_full_name = addCollProvider(
 											dbcollate_full_name,
 											server_version,
 											server_edition,
 											pg_char_to_encoding(dbencoding));
-				appendStringLiteralConn(buf, dbcollate_full_name, conn);
+				if (strcmp(dbcollate_full_name, default_collate) != 0)
+				{
+					appendPQExpBufferStr(buf, " LC_COLLATE = ");
+					appendStringLiteralConn(buf, dbcollate_full_name, conn);
+				}
 				free(dbcollate_full_name);
 			}
 

src/bin/pg_dump/t/002_pg_dump.pl

@@ -80,6 +80,7 @@
 			"--file=$tempdir/createdb.sql",
 			'-C',
 			'-R',    # no-op, just for testing
+			'--add-collprovider',     # no-op, just for testing
 			'-v',
 			'postgres', ], },
 	data_only => {
@@ -182,7 +183,8 @@
 	pg_dumpall_dbprivs => {
 		dump_cmd => [
 			'pg_dumpall', '--no-sync',
-			"--file=$tempdir/pg_dumpall_dbprivs.sql", ], },
+			"--file=$tempdir/pg_dumpall_dbprivs.sql",
+			'--add-collprovider', ], },    # no-op, just for testing
 	no_blobs => {
 		dump_cmd => [
 			'pg_dump',                      '--no-sync',