From b7739ebecdcc157be2fee348912ae3197383895f Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 11 Mar 2020 11:04:59 -0300 Subject: [PATCH] Avoid duplicates in ALTER ... DEPENDS ON EXTENSION If the command is attempted for an extension that the object already depends on, silently do nothing. In particular, this means that if a database containing multiple such entries is dumped, the restore will silently do the right thing and record just the first one. (At least, in a world where pg_dump does dump such entries -- which it doesn't currently, but it will.) Backpatch to 9.6, where this kind of dependency was introduced. Reviewed-by: Ibrar Ahmed, Tom Lane (offlist) Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql --- src/backend/catalog/pg_depend.c | 43 +++++++++++++++++++ src/backend/commands/alter.c | 7 ++- src/include/catalog/dependency.h | 1 + .../expected/test_extdepend.out | 2 + .../test_extensions/sql/test_extdepend.sql | 2 + 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 4116e93b64..9ffadbbc18 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 40ca64e226..c8f471be38 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -434,6 +434,7 @@ 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,7 +464,11 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre if (refAddress) *refAddress = refAddr; - recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION); + /* 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/include/catalog/dependency.h b/src/include/catalog/dependency.h index bee9b3ff57..9a11acfade 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, AttrNumber attnum); diff --git a/src/test/modules/test_extensions/expected/test_extdepend.out b/src/test/modules/test_extensions/expected/test_extdepend.out index 11e441ddd3..40533e90de 100644 --- a/src/test/modules/test_extensions/expected/test_extdepend.out +++ b/src/test/modules/test_extensions/expected/test_extdepend.out @@ -47,6 +47,8 @@ SELECT * FROM test_extdep_commands \gexec CREATE INDEX e ON a (a1) ALTER INDEX e DEPENDS ON EXTENSION test_ext5 RESET search_path +-- A dependent object made dependent again has no effect +ALTER FUNCTION test_ext.b() DEPENDS ON EXTENSION test_ext5; -- make sure we have the right dependencies on the extension SELECT deptype, p.* FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p diff --git a/src/test/modules/test_extensions/sql/test_extdepend.sql b/src/test/modules/test_extensions/sql/test_extdepend.sql index cf44145dcb..cc170ab709 100644 --- a/src/test/modules/test_extensions/sql/test_extdepend.sql +++ b/src/test/modules/test_extensions/sql/test_extdepend.sql @@ -27,6 +27,8 @@ COPY test_extdep_commands FROM stdin; SELECT * FROM test_extdep_commands; -- First, test that dependent objects go away when the extension is dropped. SELECT * FROM test_extdep_commands \gexec +-- A dependent object made dependent again has no effect +ALTER FUNCTION test_ext.b() DEPENDS ON EXTENSION test_ext5; -- make sure we have the right dependencies on the extension SELECT deptype, p.* FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p -- 2.39.5