Re: Extension tracking temp table and causing update failure - Mailing list pgsql-bugs

From Tom Lane
Subject Re: Extension tracking temp table and causing update failure
Date
Msg-id 15629.1331145998@sss.pgh.pa.us
Whole thread Raw
In response to Re: Extension tracking temp table and causing update failure  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Extension tracking temp table and causing update failure
Re: Extension tracking temp table and causing update failure
List pgsql-bugs
I wrote:
> But anyway, we all seem to agree that this seems like a reasonable fix,
> so I will look into making it happen.

The attached proposed patch fixes the symptom Phil reported.  However,
I think we still have some work to do.  I experimented with creating
temp tables within an extension upgrade script, and found two
interesting misbehaviors that the patch doesn't fix:

1. If you forget to drop the temp table before ending the script,
then when the session ends and the temp table is forcibly dropped,
the whole extension goes away (following the rule that a forced drop
of an extension member makes the whole extension go away).  This is
mildly annoying, but since not dropping the temp table is a clear bug
in an extension script, I think we can live with it.

2. #1 applies only in the typical case where the backend's temp table
schema already exists.  Otherwise, when we create the pg_temp_N schema
it gets listed as one of the extension's objects.  Then, when you exit
the session, this happens (behind the scenes; it's logged in the
postmaster log but not shown to the client):

FATAL:  cannot drop schema pg_temp_2 because extension myext requires it
HINT:  You can drop extension myext instead.

This is really bad: any temp tables created in this session won't be
cleaned out.  And the same for any temp tables created in future
sessions using the same backend ID slot, since they'll get the identical
error on exit.  Even worse, if you decide to drop the extension, you
might be taking out temp tables that belong to some active session other
than your own.  Given those hazards and the fact that there's no
reasonable way for an extension script to avoid the risk, I think this
one is a must-fix.

I don't see any easy way around this one other than kluging temp-schema
creation to not link the temp schema to the active extension.  Which is
exactly what I'd not wanted to do in ATRewriteTable.  The one bright spot
about it is that temp-table schemas are inherently a special case
because of their weird creation process, so we could have some comfort
that there are probably not other similar bugs lurking.

Thoughts, better ideas?

            regards, tom lane

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index db86262b4f06ec5b78f834eb10fbae45a1e77033..eca064f0cdef7ee6367b76a4da785e29a9cfdbc6 100644
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
*************** findDependentObjects(const ObjectAddress
*** 560,576 ****
                   * another object, or is part of the extension that is the
                   * other object.  We have three cases:
                   *
!                  * 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;

                      if (pendingObjects &&
                          object_address_present(&otherObject, pendingObjects))
                      {
--- 560,580 ----
                   * 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.
                   */
                  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))
                      {
*************** findDependentObjects(const ObjectAddress
*** 579,584 ****
--- 583,603 ----
                          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),

pgsql-bugs by date:

Previous
From: Merlin Moncure
Date:
Subject: Re: [GENERAL] Altering a table with a rowtype column
Next
From: Phil Sorber
Date:
Subject: Re: Extension tracking temp table and causing update failure