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

The approach used in commit bbe0a81db 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 bbe0a81db, 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

diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 0296b9bb5e2c2d468e3b79352d51e88edc2ac4ed..3bc86635f798d7a74ce5afe775f04ab2c1e60d5d 100644 (file)
--- a/src/bin/pg_dump/pg_backup.h
+++ b/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 */

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 1f82c6499be16fee5bedfa8c8adf37617fe8cefd..0ac8b187be5d5aba3ca7a214d679da2f42ef37e6 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/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");
 

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f8bec3ffcc8973d6b8534d658e687ed69b8862f8..da6cc054b08053b21e8609db511890ffac7dd38b 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/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);

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 453f9467c6ee2a572ae905ca02f853e1b9f23153..53408430819bfc69c52dffed94c9f2abc26ba534 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/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.

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index bc91bb12ac451856817a35e3bde9af7a86e14a5a..737e46464ab7ea3e917070fe0659a3ce0fd38db7 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2284,9 +2284,9 @@ my %tests = (
        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 @@ my %tests = (
        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 @@ my %tests = (
            \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 @@ my %tests = (
        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