Fix up pg_dump's handling of per-attribute compression options.

The approach used in commit bbe0a81 would've been disastrous for
portability of dumps.  Instead handle non-default compression options
in separate ALTER TABLE commands.  This reduces chatter for the
common case where most columns are compressed the same way, and it
makes it possible to restore the dump to a server that lacks any
knowledge of per-attribute compression options (so long as you're
willing to ignore syntax errors from the ALTER TABLE commands).

There's a whole lot left to do to mop up after bbe0a81, but
I'm fast-tracking this part because we need to see if it's
enough to make the buildfarm's cross-version-upgrade tests happy.

Justin Pryzby and Tom Lane

Discussion: https://postgr.es/m/20210119190720.GL8560@telsasoft.com

Author: tglsfdc

src/bin/pg_dump/pg_backup.h

@@ -159,8 +159,8 @@ typedef struct _dumpOptions
 	int			no_publications;
 	int			no_subscriptions;
 	int			no_synchronized_snapshots;
-	int			no_unlogged_table_data;
 	int			no_toast_compression;
+	int			no_unlogged_table_data;
 	int			serializable_deferrable;
 	int			disable_triggers;
 	int			outputNoTablespaces;
@@ -209,6 +209,8 @@ typedef struct Archive
 
 	/* other important stuff */
 	char	   *searchpath;		/* search_path to set during restore */
+	char	   *default_toast_compression;	/* default TOAST compression to
+											 * set during restore */
 	char	   *use_role;		/* Issue SET ROLE to this */
 
 	/* error handling */

src/bin/pg_dump/pg_backup_archiver.c

@@ -86,6 +86,7 @@ static void _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam);
 static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
 static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
 static void processSearchPathEntry(ArchiveHandle *AH, TocEntry *te);
+static void processToastCompressionEntry(ArchiveHandle *AH, TocEntry *te);
 static int	_tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH);
 static RestorePass _tocEntryRestorePass(TocEntry *te);
 static bool _tocEntryIsACL(TocEntry *te);
@@ -2696,6 +2697,8 @@ ReadToc(ArchiveHandle *AH)
 			processStdStringsEntry(AH, te);
 		else if (strcmp(te->desc, "SEARCHPATH") == 0)
 			processSearchPathEntry(AH, te);
+		else if (strcmp(te->desc, "TOASTCOMPRESSION") == 0)
+			processToastCompressionEntry(AH, te);
 	}
 }
 
@@ -2753,6 +2756,29 @@ processSearchPathEntry(ArchiveHandle *AH, TocEntry *te)
 	AH->public.searchpath = pg_strdup(te->defn);
 }
 
+static void
+processToastCompressionEntry(ArchiveHandle *AH, TocEntry *te)
+{
+	/* te->defn should have the form SET default_toast_compression = 'x'; */
+	char	   *defn = pg_strdup(te->defn);
+	char	   *ptr1;
+	char	   *ptr2 = NULL;
+
+	ptr1 = strchr(defn, '\'');
+	if (ptr1)
+		ptr2 = strchr(++ptr1, '\'');
+	if (ptr2)
+	{
+		*ptr2 = '\0';
+		AH->public.default_toast_compression = pg_strdup(ptr1);
+	}
+	else
+		fatal("invalid TOASTCOMPRESSION item: %s",
+			  te->defn);
+
+	free(defn);
+}
+
 static void
 StrictNamesCheck(RestoreOptions *ropt)
 {
@@ -2812,7 +2838,8 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
 	/* These items are treated specially */
 	if (strcmp(te->desc, "ENCODING") == 0 ||
 		strcmp(te->desc, "STDSTRINGS") == 0 ||
-		strcmp(te->desc, "SEARCHPATH") == 0)
+		strcmp(te->desc, "SEARCHPATH") == 0 ||
+		strcmp(te->desc, "TOASTCOMPRESSION") == 0)
 		return REQ_SPECIAL;
 
 	/*
@@ -3135,6 +3162,11 @@ _doSetFixedOutputState(ArchiveHandle *AH)
 	if (AH->public.searchpath)
 		ahprintf(AH, "%s", AH->public.searchpath);
 
+	/* Select the dump-time default_toast_compression */
+	if (AH->public.default_toast_compression)
+		ahprintf(AH, "SET default_toast_compression = '%s';\n",
+				 AH->public.default_toast_compression);
+
 	/* Make sure function checking is disabled */
 	ahprintf(AH, "SET check_function_bodies = false;\n");
 

src/bin/pg_dump/pg_dump.c

@@ -270,6 +270,7 @@ static void dumpDatabaseConfig(Archive *AH, PQExpBuffer outbuf,
 static void dumpEncoding(Archive *AH);
 static void dumpStdStrings(Archive *AH);
 static void dumpSearchPath(Archive *AH);
+static void dumpToastCompression(Archive *AH);
 static void binary_upgrade_set_type_oids_by_type_oid(Archive *fout,
 													 PQExpBuffer upgrade_buffer,
 													 Oid pg_type_oid,
@@ -384,10 +385,10 @@ main(int argc, char **argv)
 		{"no-comments", no_argument, &dopt.no_comments, 1},
 		{"no-publications", no_argument, &dopt.no_publications, 1},
 		{"no-security-labels", no_argument, &dopt.no_security_labels, 1},
-		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
-		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
+		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-toast-compression", no_argument, &dopt.no_toast_compression, 1},
+		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-sync", no_argument, NULL, 7},
 		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 		{"rows-per-insert", required_argument, NULL, 10},
@@ -909,10 +910,14 @@ main(int argc, char **argv)
 	 * order.
 	 */
 
-	/* First the special ENCODING, STDSTRINGS, and SEARCHPATH entries. */
+	/*
+	 * First the special entries for ENCODING, STDSTRINGS, SEARCHPATH and
+	 * TOASTCOMPRESSION.
+	 */
 	dumpEncoding(fout);
 	dumpStdStrings(fout);
 	dumpSearchPath(fout);
+	dumpToastCompression(fout);
 
 	/* The database items are always next, unless we don't want them at all */
 	if (dopt.outputCreateDB)
@@ -1048,9 +1053,9 @@ help(const char *progname)
 	printf(_("  --no-publications            do not dump publications\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
 	printf(_("  --no-subscriptions           do not dump subscriptions\n"));
-	printf(_("  --no-toast-compression      do not dump toast compression methods\n"));
 	printf(_("  --no-synchronized-snapshots  do not use synchronized snapshots in parallel jobs\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
+	printf(_("  --no-toast-compression       do not dump toast compression methods\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
 	printf(_("  --on-conflict-do-nothing     add ON CONFLICT DO NOTHING to INSERT commands\n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
@@ -3321,6 +3326,49 @@ dumpSearchPath(Archive *AH)
 	destroyPQExpBuffer(path);
 }
 
+/*
+ * dumpToastCompression: save the dump-time default TOAST compression in the
+ * archive
+ */
+static void
+dumpToastCompression(Archive *AH)
+{
+	const char *toast_compression;
+	PQExpBuffer qry;
+	PGresult   *res;
+
+	if (AH->remoteVersion < 140000 || AH->dopt->no_toast_compression)
+	{
+		/* server doesn't support compression, or we don't care */
+		return;
+	}
+
+	res = ExecuteSqlQueryForSingleRow(AH, "SHOW default_toast_compression");
+	toast_compression = PQgetvalue(res, 0, 0);
+
+	qry = createPQExpBuffer();
+	appendPQExpBufferStr(qry, "SET default_toast_compression = ");
+	appendStringLiteralAH(qry, toast_compression, AH);
+	appendPQExpBufferStr(qry, ";\n");
+
+	pg_log_info("saving default_toast_compression = %s", toast_compression);
+
+	ArchiveEntry(AH, nilCatalogId, createDumpId(),
+				 ARCHIVE_OPTS(.tag = "TOASTCOMPRESSION",
+							  .description = "TOASTCOMPRESSION",
+							  .section = SECTION_PRE_DATA,
+							  .createStmt = qry->data));
+
+	/*
+	 * Also save it in AH->default_toast_compression, in case we're doing
+	 * plain text dump.
+	 */
+	AH->default_toast_compression = pg_strdup(toast_compression);
+
+	PQclear(res);
+	destroyPQExpBuffer(qry);
+}
+
 
 /*
  * getBlobs:
@@ -8619,7 +8667,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 {
 	DumpOptions *dopt = fout->dopt;
 	PQExpBuffer q = createPQExpBuffer();
-	bool		createWithCompression;
 
 	for (int i = 0; i < numTables; i++)
 	{
@@ -8686,6 +8733,13 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			appendPQExpBufferStr(q,
 								 "0 AS attcollation,\n");
 
+		if (fout->remoteVersion >= 140000)
+			appendPQExpBuffer(q,
+							  "a.attcompression AS attcompression,\n");
+		else
+			appendPQExpBuffer(q,
+							  "'' AS attcompression,\n");
+
 		if (fout->remoteVersion >= 90200)
 			appendPQExpBufferStr(q,
 								 "pg_catalog.array_to_string(ARRAY("
@@ -8705,15 +8759,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			appendPQExpBufferStr(q,
 								 "'' AS attidentity,\n");
 
-		createWithCompression = (fout->remoteVersion >= 140000);
-
-		if (createWithCompression)
-			appendPQExpBuffer(q,
-							  "a.attcompression AS attcompression,\n");
-		else
-			appendPQExpBuffer(q,
-							  "NULL AS attcompression,\n");
-
 		if (fout->remoteVersion >= 110000)
 			appendPQExpBufferStr(q,
 								 "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
@@ -8757,9 +8802,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->attislocal = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
+		tbinfo->attcompression = (char *) pg_malloc(ntups * sizeof(char));
 		tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attmissingval = (char **) pg_malloc(ntups * sizeof(char *));
-		tbinfo->attcompression = (char *) pg_malloc(ntups * sizeof(char *));
 		tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
@@ -8786,9 +8831,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->notnull[j] = (PQgetvalue(res, j, PQfnumber(res, "attnotnull"))[0] == 't');
 			tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attoptions")));
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, PQfnumber(res, "attcollation")));
+			tbinfo->attcompression[j] = *(PQgetvalue(res, j, PQfnumber(res, "attcompression")));
 			tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attfdwoptions")));
 			tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attmissingval")));
-			tbinfo->attcompression[j] = *(PQgetvalue(res, j, PQfnumber(res, "attcompression")));
 			tbinfo->attrdefs[j] = NULL; /* fix below */
 			if (PQgetvalue(res, j, PQfnumber(res, "atthasdef"))[0] == 't')
 				hasdefaults = true;
@@ -15905,31 +15950,6 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 										  tbinfo->atttypnames[j]);
 					}
 
-					/*
-					 * Attribute compression
-					 */
-					if (!dopt->no_toast_compression &&
-						tbinfo->attcompression != NULL)
-					{
-						char	   *cmname;
-
-						switch (tbinfo->attcompression[j])
-						{
-							case 'p':
-								cmname = "pglz";
-								break;
-							case 'l':
-								cmname = "lz4";
-								break;
-							default:
-								cmname = NULL;
-								break;
-						}
-
-						if (cmname != NULL)
-							appendPQExpBuffer(q, " COMPRESSION %s", cmname);
-					}
-
 					if (print_default)
 					{
 						if (tbinfo->attgenerated[j] == ATTRIBUTE_GENERATED_STORED)
@@ -16348,7 +16368,36 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 								  qualrelname,
 								  fmtId(tbinfo->attnames[j]),
 								  tbinfo->attfdwoptions[j]);
-		}
+
+			/*
+			 * Dump per-column compression, if different from default.
+			 */
+			if (!dopt->no_toast_compression)
+			{
+				const char *cmname;
+
+				switch (tbinfo->attcompression[j])
+				{
+					case 'p':
+						cmname = "pglz";
+						break;
+					case 'l':
+						cmname = "lz4";
+						break;
+					default:
+						cmname = NULL;
+						break;
+				}
+
+				if (cmname != NULL &&
+					(fout->default_toast_compression == NULL ||
+					 strcmp(cmname, fout->default_toast_compression) != 0))
+					appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET COMPRESSION %s;\n",
+									  foreign, qualrelname,
+									  fmtId(tbinfo->attnames[j]),
+									  cmname);
+			}
+		}						/* end loop over columns */
 
 		if (ftoptions)
 			free(ftoptions);

src/bin/pg_dump/pg_dump.h

@@ -316,6 +316,7 @@ typedef struct _tableInfo
 	bool	   *attislocal;		/* true if attr has local definition */
 	char	  **attoptions;		/* per-attribute options */
 	Oid		   *attcollation;	/* per-attribute collation selection */
+	char	   *attcompression; /* per-attribute compression method */
 	char	  **attfdwoptions;	/* per-attribute fdw options */
 	char	  **attmissingval;	/* per attribute missing value */
 	bool	   *notnull;		/* NOT NULL constraints on attributes */
@@ -326,7 +327,6 @@ typedef struct _tableInfo
 	char	   *partbound;		/* partition bound definition */
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 	char	   *amname;			/* relation access method */
-	char	   *attcompression; /* per-attribute current compression method */
 
 	/*
 	 * Stuff computed only for dumpable tables.

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

@@ -2284,9 +2284,9 @@
 		regexp => qr/^
 			\QCREATE TABLE dump_test.test_table (\E\n
 			\s+\Qcol1 integer NOT NULL,\E\n
-			\s+\Qcol2 text COMPRESSION\E\D*,\n
-			\s+\Qcol3 text COMPRESSION\E\D*,\n
-			\s+\Qcol4 text COMPRESSION\E\D*,\n
+			\s+\Qcol2 text,\E\n
+			\s+\Qcol3 text,\E\n
+			\s+\Qcol4 text,\E\n
 			\s+\QCONSTRAINT test_table_col1_check CHECK ((col1 <= 1000))\E\n
 			\Q)\E\n
 			\QWITH (autovacuum_enabled='false', fillfactor='80');\E\n/xm,
@@ -2326,7 +2326,7 @@
 		regexp => qr/^
 			\QCREATE TABLE dump_test.test_second_table (\E
 			\n\s+\Qcol1 integer,\E
-			\n\s+\Qcol2 text COMPRESSION\E\D*
+			\n\s+\Qcol2 text\E
 			\n\);
 			/xm,
 		like =>
@@ -2441,7 +2441,7 @@
 			\n\s+\Qcol1 integer,\E
 			\n\s+\Qcol2 boolean,\E
 			\n\s+\Qcol3 boolean,\E
-			\n\s+\Qcol4 bit(5) COMPRESSION\E\D*,
+			\n\s+\Qcol4 bit(5),\E
 			\n\s+\Qcol5 double precision\E
 			\n\);
 			/xm,
@@ -2459,7 +2459,7 @@
 		regexp => qr/^
 			\QCREATE TABLE dump_test.test_table_identity (\E\n
 			\s+\Qcol1 integer NOT NULL,\E\n
-			\s+\Qcol2 text COMPRESSION\E\D*\n
+			\s+\Qcol2 text\E\n
 			\);
 			.*
 			\QALTER TABLE dump_test.test_table_identity ALTER COLUMN col1 ADD GENERATED ALWAYS AS IDENTITY (\E\n