more ALTER .. DEPENDS ON EXTENSION fixes
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
Attachments:
0001-Add-pg_dump-support-for-ALTER-object-DEPENDS-ON-EXTE.patchtext/x-diff; charset=us-asciiDownload
From 86cd061aff0a1c39ccfdc945a42fe88612ce2182 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@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
On 2020-Feb-17, Alvaro Herrera wrote:
* 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.
Here's these two patches. There's an "if (true)" in 0002 which is a
little weird -- that's there just to avoid reindenting those lines in
0003.
In principle, I would think that these are all backpatchable bugfixes.
Maybe 0002 could pass as not backpatchable since it disallows a command
that works today. OTOH the feature is rarely used, so maybe a backpatch
is not welcome anyhow.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0002-Avoid-duplicates-in-ALTER-.-DEPENDS-ON-EXTENSION.patchtext/x-diff; charset=us-asciiDownload
From 586899b994e45a2b2bf217a25198ba4bdbc6ee82 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 17 Feb 2020 19:42:57 -0300
Subject: [PATCH 2/3] Avoid duplicates in ALTER ... DEPENDS ON EXTENSION
If the command is attempted on an extension that the object already
depends on, raise an error.
---
src/backend/catalog/pg_depend.c | 43 ++++++++++++++++++++++++++++++++
src/backend/commands/alter.c | 17 ++++++++++++-
src/include/catalog/dependency.h | 1 +
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index f9af245eec..596dafe19c 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -648,6 +648,49 @@ getExtensionOfObject(Oid classId, Oid objectId)
return result;
}
+/*
+ * Return (possibly NIL) list of extensions that the given object depends on
+ * in DEPENDENCY_AUTO_EXTENSION mode.
+ */
+List *
+getAutoExtensionsOfObject(Oid classId, Oid objectId)
+{
+ List *result = NIL;
+ Relation depRel;
+ ScanKeyData key[2];
+ SysScanDesc scan;
+ HeapTuple tup;
+
+ depRel = table_open(DependRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(classId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objectId));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+ NULL, 2, key);
+
+ while (HeapTupleIsValid((tup = systable_getnext(scan))))
+ {
+ Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (depform->refclassid == ExtensionRelationId &&
+ depform->deptype == DEPENDENCY_AUTO_EXTENSION)
+ result = lappend_oid(result, depform->refobjid);
+ }
+
+ systable_endscan(scan);
+
+ table_close(depRel, AccessShareLock);
+
+ return result;
+}
+
/*
* Detect whether a sequence is marked as "owned" by a column
*
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 1cb84182b0..28e32f2fb5 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -462,7 +462,22 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
if (refAddress)
*refAddress = refAddr;
- recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+ if (true)
+ {
+ List *currexts;
+
+ /* Avoid duplicates */
+ currexts = getAutoExtensionsOfObject(address.classId,
+ address.objectId);
+ if (list_member_oid(currexts, refAddr.objectId))
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("%s already depends on extension \"%s\"",
+ getObjectDescription(&address),
+ get_extension_name(refAddr.objectId))));
+
+ recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+ }
return address;
}
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 0cd6fcf027..ab5e92bdc6 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -207,6 +207,7 @@ extern long changeDependenciesOn(Oid refClassId, Oid oldRefObjectId,
Oid newRefObjectId);
extern Oid getExtensionOfObject(Oid classId, Oid objectId);
+extern List *getAutoExtensionsOfObject(Oid classId, Oid objectId);
extern bool sequenceIsOwned(Oid seqId, char deptype, Oid *tableId, int32 *colId);
extern List *getOwnedSequences(Oid relid);
--
2.20.1
0003-Add-ALTER-.-NO-DEPENDS-ON.patchtext/x-diff; charset=us-asciiDownload
From 540941aa2d8a3bd84518dbd49600dc013771c275 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 30 Oct 2019 16:57:29 -0300
Subject: [PATCH 3/3] Add ALTER .. NO DEPENDS ON
This new command removes any previously added dependency mark; necessary
for completeness.
---
doc/src/sgml/ref/alter_function.sgml | 10 ++--
doc/src/sgml/ref/alter_index.sgml | 9 ++--
doc/src/sgml/ref/alter_materialized_view.sgml | 11 ++---
doc/src/sgml/ref/alter_trigger.sgml | 7 ++-
src/backend/catalog/pg_depend.c | 49 +++++++++++++++++++
src/backend/commands/alter.c | 17 ++++++-
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/equalfuncs.c | 1 +
src/backend/parser/gram.y | 36 +++++++++-----
src/include/catalog/dependency.h | 4 ++
src/include/nodes/parsenodes.h | 1 +
.../expected/test_extdepend.out | 39 ++++++++++++++-
.../test_extensions/sql/test_extdepend.sql | 18 ++++++-
13 files changed, 169 insertions(+), 34 deletions(-)
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 03ffa5945a..70b1f24bc0 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -30,7 +30,7 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) ]
SET SCHEMA <replaceable>new_schema</replaceable>
ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) ]
- DEPENDS ON EXTENSION <replaceable>extension_name</replaceable>
+ [ NO ] DEPENDS ON EXTENSION <replaceable>extension_name</replaceable>
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
@@ -153,10 +153,14 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
</varlistentry>
<varlistentry>
- <term><replaceable class="parameter">extension_name</replaceable></term>
+ <term><literal>DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
+ <term><literal>NO DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
<listitem>
<para>
- The name of the extension that the function is to depend on.
+ This form marks the function as dependent on the extension, or no longer
+ dependent on that extension if <literal>NO</literal> is specified.
+ A function that's marked as dependent on an extension is automatically
+ dropped when the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index 6d34dbb74e..de6f89d458 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -100,11 +100,14 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</varlistentry>
<varlistentry>
- <term><literal>DEPENDS ON EXTENSION</literal></term>
+ <term><literal>DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
+ <term><literal>NO DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
<listitem>
<para>
- This form marks the index as dependent on the extension, such that if the
- extension is dropped, the index will automatically be dropped as well.
+ This form marks the index as dependent on the extension, or no longer
+ dependent on that extension if <literal>NO</literal> is specified.
+ An index that's marked as dependent on an extension is automatically
+ dropped when the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_materialized_view.sgml b/doc/src/sgml/ref/alter_materialized_view.sgml
index 03e3df1ffd..9df8a79977 100644
--- a/doc/src/sgml/ref/alter_materialized_view.sgml
+++ b/doc/src/sgml/ref/alter_materialized_view.sgml
@@ -68,12 +68,6 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
anyway.)
</para>
- <para>
- The <literal>DEPENDS ON EXTENSION</literal> form marks the materialized view
- as dependent on an extension, such that the materialized view will
- automatically be dropped if the extension is dropped.
- </para>
-
<para>
The statement subforms and actions available for
<command>ALTER MATERIALIZED VIEW</command> are a subset of those available
@@ -110,7 +104,10 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
<term><replaceable class="parameter">extension_name</replaceable></term>
<listitem>
<para>
- The name of the extension that the materialized view is to depend on.
+ The name of the extension that the materialized view is to depend on (or no longer
+ dependent on, if <literal>NO</literal> is specified). A materialized view
+ that's marked as dependent on an extension is automatically dropped when
+ the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_trigger.sgml b/doc/src/sgml/ref/alter_trigger.sgml
index 6cf789a67a..6d4784c82f 100644
--- a/doc/src/sgml/ref/alter_trigger.sgml
+++ b/doc/src/sgml/ref/alter_trigger.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
-ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable>
+ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> [ NO ] DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -78,7 +78,10 @@ ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable
<term><replaceable class="parameter">extension_name</replaceable></term>
<listitem>
<para>
- The name of the extension that the trigger is to depend on.
+ The name of the extension that the trigger is to depend on (or no longer
+ dependent on, if <literal>NO</literal> is specified). A trigger
+ that's marked as dependent on an extension is automatically dropped when
+ the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 596dafe19c..fa38ee9477 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -278,6 +278,55 @@ deleteDependencyRecordsForClass(Oid classId, Oid objectId,
return count;
}
+/*
+ * deleteDependencyRecordsForSpecific -- delete all records with given depender
+ * classId/objectId, dependee classId/objectId, of the given deptype.
+ * Returns the number of records deleted.
+ */
+long
+deleteDependencyRecordsForSpecific(Oid classId, Oid objectId, char deptype,
+ Oid refclassId, Oid refobjectId)
+{
+ long count = 0;
+ Relation depRel;
+ ScanKeyData key[2];
+ SysScanDesc scan;
+ HeapTuple tup;
+
+ depRel = table_open(DependRelationId, RowExclusiveLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(classId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objectId));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+ NULL, 2, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (depform->refclassid == refclassId &&
+ depform->refobjid == refobjectId &&
+ depform->deptype == deptype)
+ {
+ CatalogTupleDelete(depRel, &tup->t_self);
+ count++;
+ }
+ }
+
+ systable_endscan(scan);
+
+ table_close(depRel, RowExclusiveLock);
+
+ return count;
+}
+
/*
* Adjust dependency record(s) to point to a different object of the same type
*
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 28e32f2fb5..4a98e11efe 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -421,7 +421,7 @@ ExecRenameStmt(RenameStmt *stmt)
}
/*
- * Executes an ALTER OBJECT / DEPENDS ON [EXTENSION] statement.
+ * Executes an ALTER OBJECT / [NO] DEPENDS ON EXTENSION statement.
*
* Return value is the address of the altered object. refAddress is an output
* argument which, if not null, receives the address of the object that the
@@ -462,10 +462,23 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
if (refAddress)
*refAddress = refAddr;
- if (true)
+ if (stmt->remove)
+ deleteDependencyRecordsForSpecific(address.classId, address.objectId,
+ DEPENDENCY_AUTO_EXTENSION,
+ refAddr.classId, refAddr.objectId);
+ else
{
+ Oid currextoid;
List *currexts;
+ /* Fail if object is part of an extension */
+ currextoid = getExtensionOfObject(address.classId, address.objectId);
+ if (OidIsValid(currextoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("%s is already part of extension \"%s\"",
+ getObjectDescription(&address),
+ get_extension_name(currextoid))));
/* Avoid duplicates */
currexts = getAutoExtensionsOfObject(address.classId,
address.objectId);
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 54ad62bb7f..851e0b0c4a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3593,6 +3593,7 @@ _copyAlterObjectDependsStmt(const AlterObjectDependsStmt *from)
COPY_NODE_FIELD(relation);
COPY_NODE_FIELD(object);
COPY_NODE_FIELD(extname);
+ COPY_SCALAR_FIELD(remove);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5b1ba143b1..8c734045a8 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1445,6 +1445,7 @@ _equalAlterObjectDependsStmt(const AlterObjectDependsStmt *a, const AlterObjectD
COMPARE_NODE_FIELD(relation);
COMPARE_NODE_FIELD(object);
COMPARE_NODE_FIELD(extname);
+ COMPARE_SCALAR_FIELD(remove);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1b0edf5d3d..d99156194d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -311,7 +311,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <list> vac_analyze_option_list
%type <node> vac_analyze_option_arg
%type <defelt> drop_option
-%type <boolean> opt_or_replace
+%type <boolean> opt_or_replace opt_no
opt_grant_grant_option opt_grant_admin_option
opt_nowait opt_if_exists opt_with_data
opt_transaction_chain
@@ -9024,57 +9024,67 @@ opt_set_data: SET DATA_P { $$ = 1; }
*****************************************************************************/
AlterObjectDependsStmt:
- ALTER FUNCTION function_with_argtypes DEPENDS ON EXTENSION name
+ ALTER FUNCTION function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_FUNCTION;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER PROCEDURE function_with_argtypes DEPENDS ON EXTENSION name
+ | ALTER PROCEDURE function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_PROCEDURE;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER ROUTINE function_with_argtypes DEPENDS ON EXTENSION name
+ | ALTER ROUTINE function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_ROUTINE;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER TRIGGER name ON qualified_name DEPENDS ON EXTENSION name
+ | ALTER TRIGGER name ON qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_TRIGGER;
n->relation = $5;
n->object = (Node *) list_make1(makeString($3));
- n->extname = makeString($9);
+ n->extname = makeString($10);
+ n->remove = $6;
$$ = (Node *)n;
}
- | ALTER MATERIALIZED VIEW qualified_name DEPENDS ON EXTENSION name
+ | ALTER MATERIALIZED VIEW qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_MATVIEW;
n->relation = $4;
- n->extname = makeString($8);
+ n->extname = makeString($9);
+ n->remove = $5;
$$ = (Node *)n;
}
- | ALTER INDEX qualified_name DEPENDS ON EXTENSION name
+ | ALTER INDEX qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_INDEX;
n->relation = $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
;
+opt_no: NO { $$ = true; }
+ | /* EMPTY */ { $$ = false; }
+ ;
+
/*****************************************************************************
*
* ALTER THING name SET SCHEMA name
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index ab5e92bdc6..2c6abe26a5 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -196,6 +196,10 @@ extern long deleteDependencyRecordsFor(Oid classId, Oid objectId,
extern long deleteDependencyRecordsForClass(Oid classId, Oid objectId,
Oid refclassId, char deptype);
+extern long deleteDependencyRecordsForSpecific(Oid classId, Oid objectId,
+ char deptype,
+ Oid refclassId, Oid refobjectId);
+
extern long changeDependencyFor(Oid classId, Oid objectId,
Oid refClassId, Oid oldRefObjectId,
Oid newRefObjectId);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index da0706add5..dad344f137 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2930,6 +2930,7 @@ typedef struct AlterObjectDependsStmt
RangeVar *relation; /* in case a table is involved */
Node *object; /* name of the object */
Value *extname; /* extension name */
+ bool remove; /* set true to remove dep rather than add */
} AlterObjectDependsStmt;
/* ----------------------
diff --git a/src/test/modules/test_extensions/expected/test_extdepend.out b/src/test/modules/test_extensions/expected/test_extdepend.out
index 11e441ddd3..7bb1d8f362 100644
--- a/src/test/modules/test_extensions/expected/test_extdepend.out
+++ b/src/test/modules/test_extensions/expected/test_extdepend.out
@@ -147,6 +147,41 @@ SELECT deptype, i.*
---------+------+--------+------+----------
(0 rows)
-DROP TABLE a;
+RESET search_path;
DROP SCHEMA test_ext CASCADE;
-NOTICE: drop cascades to extension test_ext5
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to extension test_ext5
+drop cascades to table test_ext.a
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+ CREATE SCHEMA test_ext
+ CREATE EXTENSION test_ext5 SCHEMA test_ext
+ SET search_path TO test_ext
+ CREATE TABLE a (a1 int)
+
+ CREATE FUNCTION b() RETURNS TRIGGER LANGUAGE plpgsql AS
+ $$ BEGIN NEW.a1 := NEW.a1 + 42; RETURN NEW; END; $$
+ ALTER FUNCTION b() DEPENDS ON EXTENSION test_ext5
+
+ CREATE TRIGGER c BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE b()
+ ALTER TRIGGER c ON a DEPENDS ON EXTENSION test_ext5
+
+ CREATE MATERIALIZED VIEW d AS SELECT * FROM a
+ ALTER MATERIALIZED VIEW d DEPENDS ON EXTENSION test_ext5
+
+ CREATE INDEX e ON a (a1)
+ ALTER INDEX e DEPENDS ON EXTENSION test_ext5
+ RESET search_path
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
+DROP SCHEMA test_ext CASCADE;
+NOTICE: drop cascades to table a
diff --git a/src/test/modules/test_extensions/sql/test_extdepend.sql b/src/test/modules/test_extensions/sql/test_extdepend.sql
index cf44145dcb..b709d71e96 100644
--- a/src/test/modules/test_extensions/sql/test_extdepend.sql
+++ b/src/test/modules/test_extensions/sql/test_extdepend.sql
@@ -68,6 +68,20 @@ SELECT deptype, i.*
refobjid=(SELECT oid FROM pg_extension WHERE extname='test_ext5'))
OR (refclassid='pg_class'::regclass AND refobjid='test_ext.a'::regclass)
AND NOT deptype IN ('i', 'a');
-
-DROP TABLE a;
+RESET search_path;
+DROP SCHEMA test_ext CASCADE;
+
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
DROP SCHEMA test_ext CASCADE;
--
2.20.1
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON EXTENSION" is included in the dump. However in some case not sure why "ALTER INDEX.....DEPENDS ON EXTENSION" is repeated several times in the dump?
On 2020-Feb-28, ahsan hadi wrote:
Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON EXTENSION" is included in the dump. However in some case not sure why "ALTER INDEX.....DEPENDS ON EXTENSION" is repeated several times in the dump?
Hi, thanks for testing.
Are the repeated commands for the same index, same extension? Did you
apply the same command multiple times before running pg_dump?
There was an off-list complaint that if you repeat the ALTER .. DEPENDS
for the same object on the same extension, then the same dependency is
registered multiple times. (You can search pg_depend for "deptype = 'x'"
to see that). I suppose that would lead to the line being output
multiple times by pg_dump, also. Is that what you did?
If so: Patch 0002 is supposed to fix that problem, by raising an error
if the dependency is already registered ... though it occurs to me now
that it would be more in line with custom to make the command a silent
no-op. In fact, doing that would cause old dumps (generated with
databases containing duplicated entries) to correctly restore a single
entry, without error. Therefore my inclination now is to change 0002
that way and push and backpatch it ahead of 0001.
I realize just now that I have failed to verify what happens with
partitioned indexes.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Feb 29, 2020 at 2:38 AM Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:
On 2020-Feb-28, ahsan hadi wrote:
Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in
case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON
EXTENSION" is included in the dump. However in some case not sure why
"ALTER INDEX.....DEPENDS ON EXTENSION" is repeated several times in the
dump?Hi, thanks for testing.
Are the repeated commands for the same index, same extension?
Yes same index and same extension...
Did you
apply the same command multiple times before running pg_dump?
Yes but in some cases I applied the command once and it appeared multiple
times in the dump..
There was an off-list complaint that if you repeat the ALTER .. DEPENDS
for the same object on the same extension, then the same dependency is
registered multiple times. (You can search pg_depend for "deptype = 'x'"
to see that). I suppose that would lead to the line being output
multiple times by pg_dump, also. Is that what you did?
I checked out pg_depend for "deptype='x'" the same dependency is registered
multiple times...
If so: Patch 0002 is supposed to fix that problem, by raising an error
if the dependency is already registered ... though it occurs to me now
that it would be more in line with custom to make the command a silent
no-op. In fact, doing that would cause old dumps (generated with
databases containing duplicated entries) to correctly restore a single
entry, without error. Therefore my inclination now is to change 0002
that way and push and backpatch it ahead of 0001.
Makes sense, will also try our Patch 0002.
I realize just now that I have failed to verify what happens with
partitioned indexes.
Yes I also missed this one..
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
EMAIL: mailto: ahsan.hadi@highgo.ca
On Mon, Mar 2, 2020 at 12:45 PM Ahsan Hadi <ahsan.hadi@gmail.com> wrote:
On Sat, Feb 29, 2020 at 2:38 AM Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:On 2020-Feb-28, ahsan hadi wrote:
Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in
case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON
EXTENSION" is included in the dump. However in some case not sure why
"ALTER INDEX.....DEPENDS ON EXTENSION" is repeated several times in the
dump?Hi, thanks for testing.
Are the repeated commands for the same index, same extension?
Yes same index and same extension...
You cannot do that after applying all the patches.
Did you
apply the same command multiple times before running pg_dump?Yes but in some cases I applied the command once and it appeared multiple
times in the dump..
Not for me, it works for me.
There was an off-list complaint that if you repeat the ALTER .. DEPENDS
for the same object on the same extension, then the same dependency is
registered multiple times. (You can search pg_depend for "deptype = 'x'"
to see that). I suppose that would lead to the line being output
multiple times by pg_dump, also. Is that what you did?I checked out pg_depend for "deptype='x'" the same dependency is
registered multiple times...If so: Patch 0002 is supposed to fix that problem, by raising an error
if the dependency is already registered ... though it occurs to me now
that it would be more in line with custom to make the command a silent
no-op. In fact, doing that would cause old dumps (generated with
databases containing duplicated entries) to correctly restore a single
entry, without error. Therefore my inclination now is to change 0002
that way and push and backpatch it ahead of 0001.Makes sense, will also try our Patch 0002.
I realize just now that I have failed to verify what happens with
partitioned indexes.Yes I also missed this one..
It works for partitioned indexes.
Is this intentional that there is no error when removing a non-existing
dependency?
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
It works for me
On 2020-Mar-05, Ibrar Ahmed wrote:
Is this intentional that there is no error when removing a non-existing
dependency?
Hmm, I think we can do nothing silently if nothing is called for.
So, yes, that seems to be the way it should work.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 5, 2020 at 11:38 PM Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:
On 2020-Mar-05, Ibrar Ahmed wrote:
Is this intentional that there is no error when removing a non-existing
dependency?Hmm, I think we can do nothing silently if nothing is called for.
So, yes, that seems to be the way it should work.--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
I think we need a tab-completion patch too for this.
--
Ibrar Ahmed
Attachments:
004_tab_complete.patchapplication/octet-stream; name=004_tab_complete.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b6b08d0ccb..9eb6765341 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1691,13 +1691,14 @@ psql_completion(const char *text, int start, int end)
"RENAME", "SET", "VALIDATE CONSTRAINT");
/* ALTER INDEX */
+
else if (Matches("ALTER", "INDEX"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
"UNION SELECT 'ALL IN TABLESPACE'");
/* ALTER INDEX <name> */
else if (Matches("ALTER", "INDEX", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET",
- "RESET", "ATTACH PARTITION");
+ "RESET", "ATTACH PARTITION", "DEPENDS", "NO DEPENDS");
else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH"))
COMPLETE_WITH("PARTITION");
else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH", "PARTITION"))
@@ -1743,6 +1744,10 @@ psql_completion(const char *text, int start, int end)
"buffering =", /* GiST */
"pages_per_range =", "autosummarize =" /* BRIN */
);
+ else if (Matches("ALTER", "INDEX", MatchAny, "NO", "DEPENDS"))
+ COMPLETE_WITH("ON EXTENSION");
+ else if (Matches("ALTER", "INDEX", MatchAny, "DEPENDS"))
+ COMPLETE_WITH("ON EXTENSION");
/* ALTER LANGUAGE <name> */
else if (Matches("ALTER", "LANGUAGE", MatchAny))
On 2020-Mar-05, Alvaro Herrera wrote:
On 2020-Mar-05, Ibrar Ahmed wrote:
Is this intentional that there is no error when removing a non-existing
dependency?Hmm, I think we can do nothing silently if nothing is called for.
So, yes, that seems to be the way it should work.
I pushed 0002 to all branches (9.6+), after modifying it to silently do
nothing instead of throwing an error when the dependency exists -- same
we discussed here, for the other form of the command.
I just noticed that I failed to credit Ahsan Hadi as reviewer for this
patch :-(
Thanks for reviewing. I'll see about 0001 next, also backpatched to
9.6.
I'm still not sure whether to apply 0003 (+ your tab-completion patch,
thanks for it) to backbranches or just to master. It seems legitimate
to see it as a feature addition, but OTOH the overall feature is not
complete without it ...
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I'm still not sure whether to apply 0003 (+ your tab-completion patch,
thanks for it) to backbranches or just to master. It seems legitimate
to see it as a feature addition, but OTOH the overall feature is not
complete without it ...
0003 is the command addition to allow removing such a dependency,
right? Given the lack of field demand I see no reason to risk
adding it to the back branches.
BTW, I did not like the syntax too much. "NO DEPENDS ON EXTENSION"
doesn't seem like good English. "NOT DEPENDS ON EXTENSION" is hardly
any better. The real problem with both is that an ALTER action should
be, well, an action. A grammar stickler would say that it should be
"ALTER thing DROP DEPENDENCY ON EXTENSION ext", but perhaps we could
get away with "ALTER thing DROP DEPENDS ON EXTENSION ext" to avoid
adding a new keyword. By that logic the original command should have
been "ALTER thing ADD DEPENDS ON EXTENSION ext", but I suppose it's
too late for that.
regards, tom lane
Thanks for the reviews; I pushed 0001 now, again to all branches since
9.6. Because of the previous commit, the fact that multiple statements
are emitted is not important anymore: the server will only restore the
first one, and silently ignore subsequent ones. And once you're using a
system in that state, naturally only one will be emitted by pg_dump in
all cases.
What remains on this CF item is the new feature to remove an existing
dependency. As Tom says, given the little use this feature gets it
doesn't sound worth the destabilization risk in back branches, so I'm
going to push it only to master -- but not yet.
Thanks,
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Mar-11, Tom Lane wrote:
thanks for it) to backbranches or just to master. It seems legitimate
to see it as a feature addition, but OTOH the overall feature is not
complete without it ...0003 is the command addition to allow removing such a dependency,
right? Given the lack of field demand I see no reason to risk
adding it to the back branches.
Yeah, okay.
I hereby request permission to push this patch past the feature freeze
date; it's a very small one that completes an existing feature (rather
than a complete new feature in itself), and it's not intrusive nor
likely to break anything.
BTW, I did not like the syntax too much. "NO DEPENDS ON EXTENSION"
doesn't seem like good English. "NOT DEPENDS ON EXTENSION" is hardly
any better. The real problem with both is that an ALTER action should
be, well, an action. A grammar stickler would say that it should be
"ALTER thing DROP DEPENDENCY ON EXTENSION ext", but perhaps we could
get away with "ALTER thing DROP DEPENDS ON EXTENSION ext" to avoid
adding a new keyword. By that logic the original command should have
been "ALTER thing ADD DEPENDS ON EXTENSION ext", but I suppose it's
too late for that.
I will be submitting a version with these changes shortly.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Mar-11, Tom Lane wrote:
BTW, I did not like the syntax too much. "NO DEPENDS ON EXTENSION"
doesn't seem like good English. "NOT DEPENDS ON EXTENSION" is hardly
any better. The real problem with both is that an ALTER action should
be, well, an action. A grammar stickler would say that it should be
"ALTER thing DROP DEPENDENCY ON EXTENSION ext", but perhaps we could
get away with "ALTER thing DROP DEPENDS ON EXTENSION ext" to avoid
adding a new keyword. By that logic the original command should have
been "ALTER thing ADD DEPENDS ON EXTENSION ext", but I suppose it's
too late for that.
The problem with DROP DEPENDS is alter_table_cmd, which already defines
"DROP opt_column ColId", so there's a reduce/reduce conflict for the
ALTER INDEX and ALTER MATERIALIZED VIEW forms because "depends" could be
a column name. (It works fine for ALTER FUNCTION/ROUTINE/PROCEDURE/TRIGGER
because there's no command that tries to define a conflicting DROP form
for these.)
It works if I change DEPENDS to be type_func_name_keyword (currently
unreserved_keyword), but I bet we won't like that.
(DEPENDENCY is not a keyword of any kind, so DROP DEPENDENCY require us
making it one of high reservedness, which I suspect we don't like
either).
It would also work to use a different keyword in the DROP position;
maybe REMOVE. But that's not a keyword currently.
How about ALTER .. REVOKE DEPENDS or DELETE DEPENDS? Bison is okay
with either of those forms.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
As promised, here's a rebased version of this patch -- edits pending per
discussion to decide the grammar to use.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Add-ALTER-.-NO-DEPENDS-ON.patchtext/x-diff; charset=us-asciiDownload
From 5df1613901906cff4d0b0b7e480691b65d9d2e5c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 30 Oct 2019 16:57:29 -0300
Subject: [PATCH v2 1/2] Add ALTER .. NO DEPENDS ON
This new command removes any previously added dependency mark; necessary
for completeness.
Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
Reviewed-by: ahsan hadi <ahsan.hadi@gmail.com>
Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
---
doc/src/sgml/ref/alter_function.sgml | 10 ++--
doc/src/sgml/ref/alter_index.sgml | 9 ++--
doc/src/sgml/ref/alter_materialized_view.sgml | 11 ++---
doc/src/sgml/ref/alter_trigger.sgml | 7 ++-
src/backend/catalog/pg_depend.c | 49 +++++++++++++++++++
src/backend/commands/alter.c | 24 ++++++---
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/equalfuncs.c | 1 +
src/backend/parser/gram.y | 36 +++++++++-----
src/include/catalog/dependency.h | 4 ++
src/include/nodes/parsenodes.h | 1 +
.../expected/test_extdepend.out | 39 ++++++++++++++-
.../test_extensions/sql/test_extdepend.sql | 18 ++++++-
13 files changed, 171 insertions(+), 39 deletions(-)
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 03ffa5945a..70b1f24bc0 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -30,7 +30,7 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) ]
SET SCHEMA <replaceable>new_schema</replaceable>
ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) ]
- DEPENDS ON EXTENSION <replaceable>extension_name</replaceable>
+ [ NO ] DEPENDS ON EXTENSION <replaceable>extension_name</replaceable>
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
@@ -153,10 +153,14 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
</varlistentry>
<varlistentry>
- <term><replaceable class="parameter">extension_name</replaceable></term>
+ <term><literal>DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
+ <term><literal>NO DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
<listitem>
<para>
- The name of the extension that the function is to depend on.
+ This form marks the function as dependent on the extension, or no longer
+ dependent on that extension if <literal>NO</literal> is specified.
+ A function that's marked as dependent on an extension is automatically
+ dropped when the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index 6d34dbb74e..de6f89d458 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -100,11 +100,14 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</varlistentry>
<varlistentry>
- <term><literal>DEPENDS ON EXTENSION</literal></term>
+ <term><literal>DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
+ <term><literal>NO DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable></literal></term>
<listitem>
<para>
- This form marks the index as dependent on the extension, such that if the
- extension is dropped, the index will automatically be dropped as well.
+ This form marks the index as dependent on the extension, or no longer
+ dependent on that extension if <literal>NO</literal> is specified.
+ An index that's marked as dependent on an extension is automatically
+ dropped when the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_materialized_view.sgml b/doc/src/sgml/ref/alter_materialized_view.sgml
index 03e3df1ffd..9df8a79977 100644
--- a/doc/src/sgml/ref/alter_materialized_view.sgml
+++ b/doc/src/sgml/ref/alter_materialized_view.sgml
@@ -68,12 +68,6 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
anyway.)
</para>
- <para>
- The <literal>DEPENDS ON EXTENSION</literal> form marks the materialized view
- as dependent on an extension, such that the materialized view will
- automatically be dropped if the extension is dropped.
- </para>
-
<para>
The statement subforms and actions available for
<command>ALTER MATERIALIZED VIEW</command> are a subset of those available
@@ -110,7 +104,10 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
<term><replaceable class="parameter">extension_name</replaceable></term>
<listitem>
<para>
- The name of the extension that the materialized view is to depend on.
+ The name of the extension that the materialized view is to depend on (or no longer
+ dependent on, if <literal>NO</literal> is specified). A materialized view
+ that's marked as dependent on an extension is automatically dropped when
+ the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_trigger.sgml b/doc/src/sgml/ref/alter_trigger.sgml
index 6cf789a67a..6d4784c82f 100644
--- a/doc/src/sgml/ref/alter_trigger.sgml
+++ b/doc/src/sgml/ref/alter_trigger.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
-ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable>
+ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable class="parameter">table_name</replaceable> [ NO ] DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -78,7 +78,10 @@ ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable
<term><replaceable class="parameter">extension_name</replaceable></term>
<listitem>
<para>
- The name of the extension that the trigger is to depend on.
+ The name of the extension that the trigger is to depend on (or no longer
+ dependent on, if <literal>NO</literal> is specified). A trigger
+ that's marked as dependent on an extension is automatically dropped when
+ the extension is dropped.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 596dafe19c..fa38ee9477 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -278,6 +278,55 @@ deleteDependencyRecordsForClass(Oid classId, Oid objectId,
return count;
}
+/*
+ * deleteDependencyRecordsForSpecific -- delete all records with given depender
+ * classId/objectId, dependee classId/objectId, of the given deptype.
+ * Returns the number of records deleted.
+ */
+long
+deleteDependencyRecordsForSpecific(Oid classId, Oid objectId, char deptype,
+ Oid refclassId, Oid refobjectId)
+{
+ long count = 0;
+ Relation depRel;
+ ScanKeyData key[2];
+ SysScanDesc scan;
+ HeapTuple tup;
+
+ depRel = table_open(DependRelationId, RowExclusiveLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(classId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objectId));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+ NULL, 2, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (depform->refclassid == refclassId &&
+ depform->refobjid == refobjectId &&
+ depform->deptype == deptype)
+ {
+ CatalogTupleDelete(depRel, &tup->t_self);
+ count++;
+ }
+ }
+
+ systable_endscan(scan);
+
+ table_close(depRel, RowExclusiveLock);
+
+ return count;
+}
+
/*
* Adjust dependency record(s) to point to a different object of the same type
*
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 11db9bfe92..57edea979b 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -421,7 +421,7 @@ ExecRenameStmt(RenameStmt *stmt)
}
/*
- * Executes an ALTER OBJECT / DEPENDS ON [EXTENSION] statement.
+ * Executes an ALTER OBJECT / [DROP] DEPENDS ON EXTENSION statement.
*
* Return value is the address of the altered object. refAddress is an output
* argument which, if not null, receives the address of the object that the
@@ -433,7 +433,6 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
ObjectAddress address;
ObjectAddress refAddr;
Relation rel;
- List *currexts;
address =
get_object_address_rv(stmt->objectType, stmt->relation, (List *) stmt->object,
@@ -463,11 +462,22 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
if (refAddress)
*refAddress = refAddr;
- /* Avoid duplicates */
- currexts = getAutoExtensionsOfObject(address.classId,
- address.objectId);
- if (!list_member_oid(currexts, refAddr.objectId))
- recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+ if (stmt->remove)
+ {
+ deleteDependencyRecordsForSpecific(address.classId, address.objectId,
+ DEPENDENCY_AUTO_EXTENSION,
+ refAddr.classId, refAddr.objectId);
+ }
+ else
+ {
+ List *currexts;
+
+ /* Avoid duplicates */
+ currexts = getAutoExtensionsOfObject(address.classId,
+ address.objectId);
+ if (!list_member_oid(currexts, refAddr.objectId))
+ recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+ }
return address;
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 1525c0de72..491452ae2d 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3638,6 +3638,7 @@ _copyAlterObjectDependsStmt(const AlterObjectDependsStmt *from)
COPY_NODE_FIELD(relation);
COPY_NODE_FIELD(object);
COPY_NODE_FIELD(extname);
+ COPY_SCALAR_FIELD(remove);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f34189ab5..8408c28ec6 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1449,6 +1449,7 @@ _equalAlterObjectDependsStmt(const AlterObjectDependsStmt *a, const AlterObjectD
COMPARE_NODE_FIELD(relation);
COMPARE_NODE_FIELD(object);
COMPARE_NODE_FIELD(extname);
+ COMPARE_SCALAR_FIELD(remove);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1219ac8c26..3c78f2d1b5 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -320,7 +320,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <list> vac_analyze_option_list
%type <node> vac_analyze_option_arg
%type <defelt> drop_option
-%type <boolean> opt_or_replace
+%type <boolean> opt_or_replace opt_no
opt_grant_grant_option opt_grant_admin_option
opt_nowait opt_if_exists opt_with_data
opt_transaction_chain
@@ -9053,57 +9053,67 @@ opt_set_data: SET DATA_P { $$ = 1; }
*****************************************************************************/
AlterObjectDependsStmt:
- ALTER FUNCTION function_with_argtypes DEPENDS ON EXTENSION name
+ ALTER FUNCTION function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_FUNCTION;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER PROCEDURE function_with_argtypes DEPENDS ON EXTENSION name
+ | ALTER PROCEDURE function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_PROCEDURE;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER ROUTINE function_with_argtypes DEPENDS ON EXTENSION name
+ | ALTER ROUTINE function_with_argtypes opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_ROUTINE;
n->object = (Node *) $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
- | ALTER TRIGGER name ON qualified_name DEPENDS ON EXTENSION name
+ | ALTER TRIGGER name ON qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_TRIGGER;
n->relation = $5;
n->object = (Node *) list_make1(makeString($3));
- n->extname = makeString($9);
+ n->extname = makeString($10);
+ n->remove = $6;
$$ = (Node *)n;
}
- | ALTER MATERIALIZED VIEW qualified_name DEPENDS ON EXTENSION name
+ | ALTER MATERIALIZED VIEW qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_MATVIEW;
n->relation = $4;
- n->extname = makeString($8);
+ n->extname = makeString($9);
+ n->remove = $5;
$$ = (Node *)n;
}
- | ALTER INDEX qualified_name DEPENDS ON EXTENSION name
+ | ALTER INDEX qualified_name opt_no DEPENDS ON EXTENSION name
{
AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
n->objectType = OBJECT_INDEX;
n->relation = $3;
- n->extname = makeString($7);
+ n->extname = makeString($8);
+ n->remove = $4;
$$ = (Node *)n;
}
;
+opt_no: NO { $$ = true; }
+ | /* EMPTY */ { $$ = false; }
+ ;
+
/*****************************************************************************
*
* ALTER THING name SET SCHEMA name
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index ab5e92bdc6..2c6abe26a5 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -196,6 +196,10 @@ extern long deleteDependencyRecordsFor(Oid classId, Oid objectId,
extern long deleteDependencyRecordsForClass(Oid classId, Oid objectId,
Oid refclassId, char deptype);
+extern long deleteDependencyRecordsForSpecific(Oid classId, Oid objectId,
+ char deptype,
+ Oid refclassId, Oid refobjectId);
+
extern long changeDependencyFor(Oid classId, Oid objectId,
Oid refClassId, Oid oldRefObjectId,
Oid newRefObjectId);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 518abe42c1..5e1ffafb91 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2936,6 +2936,7 @@ typedef struct AlterObjectDependsStmt
RangeVar *relation; /* in case a table is involved */
Node *object; /* name of the object */
Value *extname; /* extension name */
+ bool remove; /* set true to remove dep rather than add */
} AlterObjectDependsStmt;
/* ----------------------
diff --git a/src/test/modules/test_extensions/expected/test_extdepend.out b/src/test/modules/test_extensions/expected/test_extdepend.out
index 40533e90de..23ab5ecfa2 100644
--- a/src/test/modules/test_extensions/expected/test_extdepend.out
+++ b/src/test/modules/test_extensions/expected/test_extdepend.out
@@ -149,6 +149,41 @@ SELECT deptype, i.*
---------+------+--------+------+----------
(0 rows)
-DROP TABLE a;
+RESET search_path;
DROP SCHEMA test_ext CASCADE;
-NOTICE: drop cascades to extension test_ext5
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to extension test_ext5
+drop cascades to table test_ext.a
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+ CREATE SCHEMA test_ext
+ CREATE EXTENSION test_ext5 SCHEMA test_ext
+ SET search_path TO test_ext
+ CREATE TABLE a (a1 int)
+
+ CREATE FUNCTION b() RETURNS TRIGGER LANGUAGE plpgsql AS
+ $$ BEGIN NEW.a1 := NEW.a1 + 42; RETURN NEW; END; $$
+ ALTER FUNCTION b() DEPENDS ON EXTENSION test_ext5
+
+ CREATE TRIGGER c BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE b()
+ ALTER TRIGGER c ON a DEPENDS ON EXTENSION test_ext5
+
+ CREATE MATERIALIZED VIEW d AS SELECT * FROM a
+ ALTER MATERIALIZED VIEW d DEPENDS ON EXTENSION test_ext5
+
+ CREATE INDEX e ON a (a1)
+ ALTER INDEX e DEPENDS ON EXTENSION test_ext5
+ RESET search_path
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
+DROP SCHEMA test_ext CASCADE;
+NOTICE: drop cascades to table a
diff --git a/src/test/modules/test_extensions/sql/test_extdepend.sql b/src/test/modules/test_extensions/sql/test_extdepend.sql
index cc170ab709..54600ef56c 100644
--- a/src/test/modules/test_extensions/sql/test_extdepend.sql
+++ b/src/test/modules/test_extensions/sql/test_extdepend.sql
@@ -70,6 +70,20 @@ SELECT deptype, i.*
refobjid=(SELECT oid FROM pg_extension WHERE extname='test_ext5'))
OR (refclassid='pg_class'::regclass AND refobjid='test_ext.a'::regclass)
AND NOT deptype IN ('i', 'a');
-
-DROP TABLE a;
+RESET search_path;
+DROP SCHEMA test_ext CASCADE;
+
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
DROP SCHEMA test_ext CASCADE;
--
2.20.1
v2-0002-Add-tab-completion-for-ALTER-INDEX-.-NO-DEPENDS-O.patchtext/x-diff; charset=us-asciiDownload
From 5ed871d60073a71795ebcd14f033b8745921bcf5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 9 Apr 2020 18:57:53 -0400
Subject: [PATCH v2 2/2] Add tab-completion for ALTER INDEX .. [NO] DEPENDS ON
We'd like to have tab-completion for the other object types too, but
they don't have sub-command completion yet.
Author: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Discussion: https://postgr.es/m/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com
---
src/bin/psql/tab-complete.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0e7a373caf..f6fd623c98 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1709,7 +1709,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER INDEX <name> */
else if (Matches("ALTER", "INDEX", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET",
- "RESET", "ATTACH PARTITION");
+ "RESET", "ATTACH PARTITION", "DEPENDS", "NO DEPENDS");
else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH"))
COMPLETE_WITH("PARTITION");
else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH", "PARTITION"))
@@ -1755,6 +1755,10 @@ psql_completion(const char *text, int start, int end)
"buffering =", /* GiST */
"pages_per_range =", "autosummarize =" /* BRIN */
);
+ else if (Matches("ALTER", "INDEX", MatchAny, "NO", "DEPENDS"))
+ COMPLETE_WITH("ON EXTENSION");
+ else if (Matches("ALTER", "INDEX", MatchAny, "DEPENDS"))
+ COMPLETE_WITH("ON EXTENSION");
/* ALTER LANGUAGE <name> */
else if (Matches("ALTER", "LANGUAGE", MatchAny))
--
2.20.1
I pushed this (to pg13 only) using the originally proposed "NO DEPENDS"
syntax. It's trivial to change to REVOKE DEPENDS on REMOVE DEPENDS if
we decide to do that.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Mar-06, Ibrar Ahmed wrote:
I think we need a tab-completion patch too for this.
Thanks, I pushed this.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services