ALTER ... DEPENDS ON EXTENSION (dependencies of type 'x' on an
extension) was found to have a few problems. One was fixed as
CVE-2020-1720. Other issues:
* pg_dump does not reproduce database state correctly.
The attached 0000 fixes it.
* More than one 'x' dependencies are allowed for the same object on the
same extension. That's useless and polluting, so should be prevented.
* There's no way to remove an 'x' dependency.
I'll send patches for the other two issues as replies later. (I
discovered an issue in my 0001, for the second one, just as I was
sending.)
--
Álvaro Herrera
>From 86cd061aff0a1c39ccfdc945a42fe88612ce2182 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Tue, 29 Oct 2019 23:19:13 -0300
Subject: [PATCH] Add pg_dump support for ALTER <object> DEPENDS ON EXTENSION
Dumping/restoring objects so marked would lose the markings, forcing
users to re-add them manually after the fact. (This affects pg_upgrade
too). Have pg_dump emit commands to preserve them.
---
src/bin/pg_dump/common.c | 1 +
src/bin/pg_dump/pg_dump.c | 107 ++++++++++++++++++--
src/bin/pg_dump/pg_dump.h | 1 +
src/test/modules/test_pg_dump/t/001_base.pl | 32 ++++++
4 files changed, 133 insertions(+), 8 deletions(-)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 69508ec9b3..4ea59f5494 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -549,6 +549,7 @@ AssignDumpId(DumpableObject *dobj)
dobj->namespace = NULL; /* may be set later */
dobj->dump = DUMP_COMPONENT_ALL; /* default assumption */
dobj->ext_member = false; /* default assumption */
+ dobj->depends_on_ext = false; /* default assumption */
dobj->dependencies = NULL;
dobj->nDeps = 0;
dobj->allocDeps = 0;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec3e2c63b0..eeb0d90b17 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4275,6 +4275,55 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
free(qsubname);
}
+/*
+ * Given a "create query", append as many ALTER ... DEPENDS ON EXTENSION as
+ * the object needs.
+ */
+static void
+append_depends_on_extension(Archive *fout,
+ PQExpBuffer create,
+ DumpableObject *dobj,
+ const char *catalog,
+ const char *keyword,
+ const char *objname)
+{
+ if (dobj->depends_on_ext)
+ {
+ char *nm;
+ PGresult *res;
+ PQExpBuffer query;
+ int ntups;
+ int i_extname;
+ int i;
+
+ /* dodge fmtId() non-reentrancy */
+ nm = pg_strdup(objname);
+
+ query = createPQExpBuffer();
+ appendPQExpBuffer(query,
+ "SELECT e.extname "
+ "FROM pg_catalog.pg_depend d, pg_catalog.pg_extension e "
+ "WHERE d.refobjid = e.oid AND classid = '%s'::pg_catalog.regclass "
+ "AND objid = '%u'::pg_catalog.oid AND deptype = 'x' "
+ "AND refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass",
+ catalog,
+ dobj->catId.oid);
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ ntups = PQntuples(res);
+ i_extname = PQfnumber(res, "extname");
+ for (i = 0; i < ntups; i++)
+ {
+ appendPQExpBuffer(create, "ALTER %s %s DEPENDS ON EXTENSION %s;\n",
+ keyword, nm,
+ fmtId(PQgetvalue(res, i, i_extname)));
+ }
+
+ PQclear(res);
+ pg_free(nm);
+ }
+}
+
+
static void
binary_upgrade_set_type_oids_by_type_oid(Archive *fout,
PQExpBuffer upgrade_buffer,
@@ -12145,6 +12194,12 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, "\n %s;\n", asPart->data);
+ append_depends_on_extension(fout, q, &finfo->dobj,
+ "pg_catalog.pg_proc", keyword,
+ psprintf("%s.%s",
+ fmtId(finfo->dobj.namespace->dobj.name),
+ funcsig));
+
if (dopt->binary_upgrade)
binary_upgrade_extension_member(q, &finfo->dobj,
keyword, funcsig,
@@ -15855,6 +15910,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
else
appendPQExpBufferStr(q, ";\n");
+ /* Materialized views can depend on extensions */
+ if (tbinfo->relkind == RELKIND_MATVIEW)
+ append_depends_on_extension(fout, q, &tbinfo->dobj,
+ "pg_catalog.pg_class",
+ tbinfo->relkind == RELKIND_MATVIEW ?
+ "MATERIALIZED VIEW" : "INDEX",
+ qualrelname);
+
/*
* in binary upgrade mode, update the catalog with any missing values
* that might be present.
@@ -16359,6 +16422,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
PQExpBuffer q;
PQExpBuffer delq;
char *qindxname;
+ char *qqindxname;
if (dopt->dataOnly)
return;
@@ -16367,6 +16431,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
delq = createPQExpBuffer();
qindxname = pg_strdup(fmtId(indxinfo->dobj.name));
+ qqindxname = pg_strdup(fmtQualifiedDumpable(indxinfo));
/*
* If there's an associated constraint, don't dump the index per se, but
@@ -16419,8 +16484,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
for (j = 0; j < nstatcols; j++)
{
- appendPQExpBuffer(q, "ALTER INDEX %s ",
- fmtQualifiedDumpable(indxinfo));
+ appendPQExpBuffer(q, "ALTER INDEX %s ", qqindxname);
/*
* Note that this is a column number, so no quotes should be
@@ -16433,6 +16497,11 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
}
}
+ /* Indexes can depend on extensions */
+ append_depends_on_extension(fout, q, &indxinfo->dobj,
+ "pg_catalog.pg_class",
+ "INDEX", qqindxname);
+
/* If the index defines identity, we need to record that. */
if (indxinfo->indisreplident)
{
@@ -16443,8 +16512,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
qindxname);
}
- appendPQExpBuffer(delq, "DROP INDEX %s;\n",
- fmtQualifiedDumpable(indxinfo));
+ appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId,
@@ -16475,6 +16543,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
destroyPQExpBuffer(q);
destroyPQExpBuffer(delq);
free(qindxname);
+ free(qqindxname);
}
/*
@@ -16713,6 +16782,11 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
fmtId(indxinfo->dobj.name));
}
+ /* Indexes can depend on extensions */
+ append_depends_on_extension(fout, q, &indxinfo->dobj,
+ "pg_catalog.pg_class", "INDEX",
+ fmtQualifiedDumpable(indxinfo));
+
appendPQExpBuffer(delq, "ALTER TABLE ONLY %s ",
fmtQualifiedDumpable(tbinfo));
appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
@@ -17232,6 +17306,7 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
PQExpBuffer query;
PQExpBuffer delqry;
PQExpBuffer trigprefix;
+ PQExpBuffer trigidentity;
char *qtabname;
char *tgargs;
size_t lentgargs;
@@ -17249,13 +17324,14 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
query = createPQExpBuffer();
delqry = createPQExpBuffer();
trigprefix = createPQExpBuffer();
+ trigidentity = createPQExpBuffer();
qtabname = pg_strdup(fmtId(tbinfo->dobj.name));
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
- fmtId(tginfo->dobj.name));
- appendPQExpBuffer(delqry, "ON %s;\n",
- fmtQualifiedDumpable(tbinfo));
+ appendPQExpBuffer(trigidentity, "%s ", fmtId(tginfo->dobj.name));
+ appendPQExpBuffer(trigidentity, "ON %s", fmtQualifiedDumpable(tbinfo));
+
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s;\n", trigidentity->data);
if (tginfo->tgdef)
{
@@ -17374,6 +17450,11 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
appendPQExpBufferStr(query, ");\n");
}
+ /* Triggers can depend on extensions */
+ append_depends_on_extension(fout, query, &tginfo->dobj,
+ "pg_catalog.pg_trigger", "TRIGGER",
+ trigidentity->data);
+
if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O')
{
appendPQExpBuffer(query, "\nALTER TABLE %s ",
@@ -17422,6 +17503,7 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
destroyPQExpBuffer(query);
destroyPQExpBuffer(delqry);
destroyPQExpBuffer(trigprefix);
+ destroyPQExpBuffer(trigidentity);
free(qtabname);
}
@@ -18071,6 +18153,15 @@ getDependencies(Archive *fout)
continue;
}
+ /*
+ * For 'x' dependencies, mark the object for later; we still add the
+ * normal dependency, for possible ordering purposes. Currently
+ * pg_dump_sort.c knows to put extensions ahead of all object types
+ * that could possibly depend on them, but this is safer.
+ */
+ if (deptype == 'x')
+ dobj->depends_on_ext = true;
+
/*
* Ordinarily, table rowtypes have implicit dependencies on their
* tables. However, for a composite type the implicit dependency goes
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 21004e5078..585a1f9257 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -132,6 +132,7 @@ typedef struct _dumpableObject
DumpComponents dump; /* bitmask of components to dump */
DumpComponents dump_contains; /* as above, but for contained objects */
bool ext_member; /* true if object is member of extension */
+ bool depends_on_ext; /* true if object depends on an extension */
DumpId *dependencies; /* dumpIds of objects this one depends on */
int nDeps; /* number of valid dependencies */
int allocDeps; /* allocated size of dependencies[] */
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index fb4ecf8aca..ae120a5ee3 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -523,6 +523,38 @@ my %tests = (
like => { binary_upgrade => 1, },
},
+ 'ALTER INDEX pkey DEPENDS ON extension' => {
+ create_order => 11,
+ create_sql =>
+ 'CREATE TABLE regress_pg_dump_schema.extdependtab (col1 integer primary key, col2 int);
+ CREATE INDEX ON regress_pg_dump_schema.extdependtab (col2);
+ ALTER INDEX regress_pg_dump_schema.extdependtab_col2_idx DEPENDS ON EXTENSION test_pg_dump;
+ ALTER INDEX regress_pg_dump_schema.extdependtab_pkey DEPENDS ON EXTENSION test_pg_dump;',
+ regexp => qr/^
+ \QALTER INDEX regress_pg_dump_schema.extdependtab_pkey DEPENDS ON EXTENSION test_pg_dump;\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ pg_dumpall_globals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ },
+ },
+
+ 'ALTER INDEX idx DEPENDS ON extension' => {
+ regexp => qr/^
+ \QALTER INDEX regress_pg_dump_schema.extdependtab_col2_idx DEPENDS ON EXTENSION test_pg_dump;\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ pg_dumpall_globals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ },
+ },
+
# Objects not included in extension, part of schema created by extension
'CREATE TABLE regress_pg_dump_schema.external_tab' => {
create_order => 4,
--
2.20.1
