Fix test about ignoring extension dependencies during extension scripts.

Commit 08dd23cec introduced an exception to the rule that extension member
objects can only be dropped as part of dropping the whole extension,
intending to allow such drops while running the extension's own creation or
update scripts.  However, the exception was only applied at the outermost
recursion level, because it was modeled on a pre-existing check to ignore
dependencies on objects listed in pendingObjects.  Bug #14434 from Philippe
Beaudoin shows that this is inadequate: in some cases we can reach an
extension member object by recursion from another one.  (The bug concerns
the serial-sequence case; I'm not sure if there are other cases, but there
might well be.)

To fix, revert 08dd23cec's changes to findDependentObjects() and instead
apply the creating_extension exception regardless of stack level.

Having seen this example, I'm a bit suspicious that the pendingObjects
logic is also wrong and such cases should likewise be allowed at any
recursion level.  However, changing that would interact in subtle ways
with the recursion logic (at least it would need to be moved to after the
recursing-from check).  Given that the code's been like that a long time,
I'll refrain from touching it without a clear example showing it's wrong.

Back-patch to all active branches.  In HEAD and 9.6, where suitable
test infrastructure exists, add a regression test case based on the
bug report.

Report: <20161125151448.6529.33039@wrigleys.postgresql.org>
Discussion: <13224.1480177514@sss.pgh.pa.us>

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 04d78402903074f68465448d58c636252e68b852..f71d80fc1a1fc4a41a4023c1ebafc51908e04f4f 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -594,29 +594,43 @@ findDependentObjects(const ObjectAddress *object,
                        case DEPENDENCY_AUTO_EXTENSION:
                                /* no problem */
                                break;
-                       case DEPENDENCY_INTERNAL:
+
                        case DEPENDENCY_EXTENSION:
 
+                               /*
+                                * If the other object is the extension currently being
+                                * created/altered, ignore this dependency and continue with
+                                * the deletion.  This allows dropping of an extension's
+                                * objects within the extension's scripts, as well as corner
+                                * cases such as dropping a transient object created within
+                                * such a script.
+                                */
+                               if (creating_extension &&
+                                       otherObject.classId == ExtensionRelationId &&
+                                       otherObject.objectId == CurrentExtensionObject)
+                                       break;
+
+                               /* Otherwise, treat this like an internal dependency */
+                               /* FALL THRU */
+
+                       case DEPENDENCY_INTERNAL:
+
                                /*
                                 * This object is part of the internal implementation of
                                 * another object, or is part of the extension that is the
                                 * other object.  We have three cases:
                                 *
-                                * 1. At the outermost recursion level, we normally disallow
-                                * the DROP.  (We just ereport here, rather than proceeding,
-                                * since no other dependencies are likely to be interesting.)
-                                * However, there are exceptions.
+                                * 1. At the outermost recursion level, disallow the DROP. (We
+                                * just ereport here, rather than proceeding, since no other
+                                * dependencies are likely to be interesting.)  However, if
+                                * the owning object is listed in pendingObjects, just release
+                                * the caller's lock and return; we'll eventually complete the
+                                * DROP when we reach that entry in the pending list.
                                 */
                                if (stack == NULL)
                                {
                                        char       *otherObjDesc;
 
-                                       /*
-                                        * Exception 1a: if the owning object is listed in
-                                        * pendingObjects, just release the caller's lock and
-                                        * return.  We'll eventually complete the DROP when we
-                                        * reach that entry in the pending list.
-                                        */
                                        if (pendingObjects &&
                                                object_address_present(&otherObject, pendingObjects))
                                        {
@@ -625,21 +639,6 @@ findDependentObjects(const ObjectAddress *object,
                                                ReleaseDeletionLock(object);
                                                return;
                                        }
-
-                                       /*
-                                        * Exception 1b: if the owning object is the extension
-                                        * currently being created/altered, it's okay to continue
-                                        * with the deletion.  This allows dropping of an
-                                        * extension's objects within the extension's scripts, as
-                                        * well as corner cases such as dropping a transient
-                                        * object created within such a script.
-                                        */
-                                       if (creating_extension &&
-                                               otherObject.classId == ExtensionRelationId &&
-                                               otherObject.objectId == CurrentExtensionObject)
-                                               break;
-
-                                       /* No exception applies, so throw the error */
                                        otherObjDesc = getObjectDescription(&otherObject);
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),

diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 8f0eeb7d3c9a551efe42a91d3f127143b967b41e..b184570779c66750392d3a5c2d1a305984235283 100644 (file)
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -4,10 +4,11 @@ MODULE = test_extensions
 PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
 
 EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
-                       test_ext_cyclic1 test_ext_cyclic2
+            test_ext7 test_ext_cyclic1 test_ext_cyclic2
 DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
-          test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
-          test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql
+       test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
+       test_ext7--1.0.sql test_ext7--1.0--2.0.sql \
+       test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql
 
 REGRESS = test_extensions test_extdepend
 

diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index c3360c97cc5745ce2eee832e32a845ca3aeac1e5..ea096b9fb618cbf20b3fb92151e7899dbd83a389 100644 (file)
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -38,3 +38,28 @@ drop cascades to extension test_ext1
 CREATE EXTENSION test_ext6;
 DROP EXTENSION test_ext6;
 CREATE EXTENSION test_ext6;
+-- test dropping of member tables that own extensions:
+-- this table will be absorbed into test_ext7
+create table old_table1 (col1 serial primary key);
+create extension test_ext7;
+\dx+ test_ext7
+Objects in extension "test_ext7"
+      Object Description       
+-------------------------------
+ sequence ext7_table1_col1_seq
+ sequence ext7_table2_col2_seq
+ sequence old_table1_col1_seq
+ table ext7_table1
+ table ext7_table2
+ table old_table1
+(6 rows)
+
+alter extension test_ext7 update to '2.0';
+\dx+ test_ext7
+Objects in extension "test_ext7"
+      Object Description       
+-------------------------------
+ sequence ext7_table2_col2_seq
+ table ext7_table2
+(2 rows)
+

diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 2ab8b599dc29587b280dd59d13d98c982327b3bc..b53be00c4e38f6fa1dbd19cda01a8aabe9fbea6c 100644 (file)
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -17,3 +17,11 @@ DROP SCHEMA test_ext CASCADE;
 CREATE EXTENSION test_ext6;
 DROP EXTENSION test_ext6;
 CREATE EXTENSION test_ext6;
+
+-- test dropping of member tables that own extensions:
+-- this table will be absorbed into test_ext7
+create table old_table1 (col1 serial primary key);
+create extension test_ext7;
+\dx+ test_ext7
+alter extension test_ext7 update to '2.0';
+\dx+ test_ext7

diff --git a/src/test/modules/test_extensions/test_ext7--1.0--2.0.sql b/src/test/modules/test_extensions/test_ext7--1.0--2.0.sql
new file mode 100644 (file)
index 0000000..50e3dca
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext7--1.0--2.0.sql
@@ -0,0 +1,8 @@
+/* src/test/modules/test_extensions/test_ext7--1.0--2.0.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext7 UPDATE TO '2.0'" to load this file. \quit
+
+-- drop some tables with serial columns
+drop table ext7_table1;
+drop table old_table1;

diff --git a/src/test/modules/test_extensions/test_ext7--1.0.sql b/src/test/modules/test_extensions/test_ext7--1.0.sql
new file mode 100644 (file)
index 0000000..0c2d72a
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext7--1.0.sql
@@ -0,0 +1,13 @@
+/* src/test/modules/test_extensions/test_ext7--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext7" to load this file. \quit
+
+-- link some existing serial-owning table to the extension
+alter extension test_ext7 add table old_table1;
+alter extension test_ext7 add sequence old_table1_col1_seq;
+
+-- ordinary member tables with serial columns
+create table ext7_table1 (col1 serial primary key);
+
+create table ext7_table2 (col2 serial primary key);

diff --git a/src/test/modules/test_extensions/test_ext7.control b/src/test/modules/test_extensions/test_ext7.control
new file mode 100644 (file)
index 0000000..b58df53
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext7.control
@@ -0,0 +1,4 @@
+comment = 'Test extension 7'
+default_version = '1.0'
+schema = 'public'
+relocatable = false