Fix possible future cache reference leak in ALTER EXTENSION ADD/DROP.

recordExtObjInitPriv and removeExtObjInitPriv were sloppy about
calling ReleaseSysCache.  The cases cannot occur given current usage
in ALTER EXTENSION ADD/DROP, since we wouldn't get here for these
relkinds; but it seems wise to clean up better.

In passing, extend test logic in test_pg_dump to exercise the
dropped-column code paths here.

Since the case is unreachable at present, there seems no great
need to back-patch; hence fix HEAD only.

Kyotaro Horiguchi, with test case and comment adjustments by me

Discussion: https://postgr.es/m/20200417.151831.1153577605111650154.horikyota.ntt@gmail.com

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index a3f680d388a4e40a01cc38c542cc9ba4f41753f0..06687c53a60f6e96a85015b1959761bb8b177a1f 100644 (file)
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -5580,19 +5580,22 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
                        elog(ERROR, "cache lookup failed for relation %u", objoid);
                pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
 
-               /* Indexes don't have permissions */
+               /*
+                * Indexes don't have permissions, neither do the pg_class rows for
+                * composite types.  (These cases are unreachable given the
+                * restrictions in ALTER EXTENSION ADD, but let's check anyway.)
+                */
                if (pg_class_tuple->relkind == RELKIND_INDEX ||
-                       pg_class_tuple->relkind == RELKIND_PARTITIONED_INDEX)
-                       return;
-
-               /* Composite types don't have permissions either */
-               if (pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+                       pg_class_tuple->relkind == RELKIND_PARTITIONED_INDEX ||
+                       pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+               {
+                       ReleaseSysCache(tuple);
                        return;
+               }
 
                /*
-                * If this isn't a sequence, index, or composite type then it's
-                * possibly going to have columns associated with it that might have
-                * ACLs.
+                * If this isn't a sequence then it's possibly going to have
+                * column-level ACLs associated with it.
                 */
                if (pg_class_tuple->relkind != RELKIND_SEQUENCE)
                {
@@ -5724,6 +5727,11 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
                SysScanDesc scan;
                Relation        relation;
 
+               /*
+                * Note: this is dead code, given that we don't allow large objects to
+                * be made extension members.  But it seems worth carrying in case
+                * some future caller of this function has need for it.
+                */
                relation = table_open(LargeObjectMetadataRelationId, RowExclusiveLock);
 
                /* There's no syscache for pg_largeobject_metadata */
@@ -5866,19 +5874,22 @@ removeExtObjInitPriv(Oid objoid, Oid classoid)
                        elog(ERROR, "cache lookup failed for relation %u", objoid);
                pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
 
-               /* Indexes don't have permissions */
+               /*
+                * Indexes don't have permissions, neither do the pg_class rows for
+                * composite types.  (These cases are unreachable given the
+                * restrictions in ALTER EXTENSION DROP, but let's check anyway.)
+                */
                if (pg_class_tuple->relkind == RELKIND_INDEX ||
-                       pg_class_tuple->relkind == RELKIND_PARTITIONED_INDEX)
-                       return;
-
-               /* Composite types don't have permissions either */
-               if (pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+                       pg_class_tuple->relkind == RELKIND_PARTITIONED_INDEX ||
+                       pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+               {
+                       ReleaseSysCache(tuple);
                        return;
+               }
 
                /*
-                * If this isn't a sequence, index, or composite type then it's
-                * possibly going to have columns associated with it that might have
-                * ACLs.
+                * If this isn't a sequence then it's possibly going to have
+                * column-level ACLs associated with it.
                 */
                if (pg_class_tuple->relkind != RELKIND_SEQUENCE)
                {

diff --git a/src/test/modules/test_pg_dump/README b/src/test/modules/test_pg_dump/README
index e6c78b822c3e38e94c9ec79a4e1009a3b7e691ca..b7c2e337ca7cb7517f70e9658fc562b7402d5895 100644 (file)
--- a/src/test/modules/test_pg_dump/README
+++ b/src/test/modules/test_pg_dump/README
@@ -1,2 +1,4 @@
 test_pg_dump is an extension explicitly to test pg_dump when
 extensions are present in the system.
+
+We also make use of this module to test ALTER EXTENSION ADD/DROP.

diff --git a/src/test/modules/test_pg_dump/expected/test_pg_dump.out b/src/test/modules/test_pg_dump/expected/test_pg_dump.out
index c9bc6f762588233ab26c037f1a34051f01ddc647..a50eaf6125dca66a04a547ea24bb79b8d61da0b4 100644 (file)
--- a/src/test/modules/test_pg_dump/expected/test_pg_dump.out
+++ b/src/test/modules/test_pg_dump/expected/test_pg_dump.out
@@ -4,7 +4,8 @@ ALTER EXTENSION test_pg_dump ADD DATABASE postgres; -- error
 ERROR:  syntax error at or near "DATABASE"
 LINE 1: ALTER EXTENSION test_pg_dump ADD DATABASE postgres;
                                          ^
-CREATE TABLE test_pg_dump_t1 (c1 int);
+CREATE TABLE test_pg_dump_t1 (c1 int, junk text);
+ALTER TABLE test_pg_dump_t1 DROP COLUMN junk;  -- to exercise dropped-col cases
 CREATE VIEW test_pg_dump_v1 AS SELECT * FROM test_pg_dump_t1;
 CREATE MATERIALIZED VIEW test_pg_dump_mv1 AS SELECT * FROM test_pg_dump_t1;
 CREATE SCHEMA test_pg_dump_s1;

diff --git a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql
index e463dec40402babdbaa7449c3fd0bad6fc6b7088..a61a7c8c4ce9557b744c135c5541c5af2976ece6 100644 (file)
--- a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql
+++ b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql
@@ -3,7 +3,8 @@ CREATE EXTENSION test_pg_dump;
 
 ALTER EXTENSION test_pg_dump ADD DATABASE postgres; -- error
 
-CREATE TABLE test_pg_dump_t1 (c1 int);
+CREATE TABLE test_pg_dump_t1 (c1 int, junk text);
+ALTER TABLE test_pg_dump_t1 DROP COLUMN junk;  -- to exercise dropped-col cases
 CREATE VIEW test_pg_dump_v1 AS SELECT * FROM test_pg_dump_t1;
 CREATE MATERIALIZED VIEW test_pg_dump_mv1 AS SELECT * FROM test_pg_dump_t1;
 CREATE SCHEMA test_pg_dump_s1;