From aa25d1089ac00bbc3f97d2efe8f54c3d4beed5d1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 20 Mar 2021 15:01:10 -0400 Subject: [PATCH] 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 --- src/bin/pg_dump/pg_backup.h | 4 +- src/bin/pg_dump/pg_backup_archiver.c | 34 ++++++- src/bin/pg_dump/pg_dump.c | 133 ++++++++++++++++++--------- src/bin/pg_dump/pg_dump.h | 2 +- src/bin/pg_dump/t/002_pg_dump.pl | 12 +-- 5 files changed, 134 insertions(+), 51 deletions(-) diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 0296b9bb5e..3bc86635f7 100644 --- 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 1f82c6499b..0ac8b187be 100644 --- 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 f8bec3ffcc..da6cc054b0 100644 --- 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 453f9467c6..5340843081 100644 --- 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 bc91bb12ac..737e46464a 100644 --- 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