On Sat, Mar 20, 2021 at 8:11 AM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Fri, Mar 19, 2021 at 8:25 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> > Extrapolating from the way we've dealt with similar issues
> > in the past, I think the structure of pg_dump's output ought to be:
> >
> > 1. SET default_toast_compression = 'source system's value'
> > in among the existing passel of SETs at the top. Doesn't
> > matter whether or not that is the compiled-in value.
> >
> > 2. No mention of compression in any CREATE TABLE command.
> >
> > 3. For any column having a compression option different from
> > the default, emit ALTER TABLE SET ... to set that option after
> > the CREATE TABLE. (You did implement such a SET, I trust.)
>
> Actually, *I* didn't implement any of this. But ALTER TABLE sometab
> ALTER somecol SET COMPRESSION somealgo works.
>
> This sounds like a reasonable approach.
The attached patch implements that.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From 40b53e24932b1f9203092a7f6972804af5a7a45b Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Sat, 20 Mar 2021 13:14:39 +0530
Subject: [PATCH] Fixup, dump toast compression method
---
src/bin/pg_dump/pg_backup.h | 1 +
src/bin/pg_dump/pg_backup_archiver.c | 4 +++
src/bin/pg_dump/pg_dump.c | 66 ++++++++++++++++++++++--------------
src/bin/pg_dump/t/002_pg_dump.pl | 12 +++----
4 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 0296b9b..02019fc 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -210,6 +210,7 @@ typedef struct Archive
/* other important stuff */
char *searchpath; /* search_path to set during restore */
char *use_role; /* Issue SET ROLE to this */
+ char *default_toast_compression;
/* error handling */
bool exit_on_error; /* whether to exit on SQL errors... */
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 1f82c64..a25f4d1 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3152,6 +3152,10 @@ _doSetFixedOutputState(ArchiveHandle *AH)
else
ahprintf(AH, "SET row_security = off;\n");
+ /* 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);
ahprintf(AH, "\n");
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f8bec3f..7bbcaac 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1248,6 +1248,21 @@ setup_connection(Archive *AH, const char *dumpencoding,
AH->sync_snapshot_id = get_synchronized_snapshot(AH);
}
+
+ /*
+ * Get default TOAST compression method, but not if the server's too
+ * old to support the feature or if the user doesn't want to dump that
+ * information anyway.
+ */
+ if (AH->remoteVersion >= 140000 && !dopt->no_toast_compression)
+ {
+ PGresult *res;
+
+ res = ExecuteSqlQueryForSingleRow(AH,
+ "SHOW default_toast_compression");
+ AH->default_toast_compression = pg_strdup(PQgetvalue(res, 0, 0));
+ PQclear(res);
+ }
}
/* Set up connection for a parallel worker process */
@@ -15905,31 +15920,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,6 +16338,32 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
qualrelname,
fmtId(tbinfo->attnames[j]),
tbinfo->attfdwoptions[j]);
+
+ /*
+ * Dump per-column compression options
+ */
+ 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 &&
+ strcmp(cmname, fout->default_toast_compression) != 0)
+ appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s;\n",
+ qualrelname, fmtId(tbinfo->attnames[j]), cmname);
+ }
}
if (ftoptions)
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index bc91bb1..737e464 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
--
1.8.3.1
