Re: Assert failure due to "drop schema pg_temp_3 cascade" for temporary tables and \d+ is not showing any info after drooping temp table schema - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Assert failure due to "drop schema pg_temp_3 cascade" for temporary tables and \d+ is not showing any info after drooping temp table schema
Date
Msg-id 4172.1582915529@sss.pgh.pa.us
Whole thread Raw
In response to Re: Assert failure due to "drop schema pg_temp_3 cascade" for temporary tables and \d+ is not showing any info after drooping temp table schema  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Assert failure due to "drop schema pg_temp_3 cascade" fortemporary tables and \d+ is not showing any info after drooping temp tableschema
List pgsql-hackers
I wrote:
> Also, I notice that isTempNamespaceInUse is already detecting the case
> where the namespace doesn't exist or isn't really a temp namespace.
> I wonder whether it'd be better to teach that to return an indicator about
> the namespace not being what you think it is.  That would force us to look
> at its other callers to see if any of them have related bugs, which seems
> like a good thing to check --- and even if they don't, having to think
> about the point in future call sites might forestall new bugs.

After poking around, I see there aren't any other callers.  But I think
that the cause of this bug is clearly failure to think carefully about
the different cases that isTempNamespaceInUse is recognizing, so that
the right way to fix it is more like the attached.

In the back branches, we could leave isTempNamespaceInUse() in place
but unused, just in case somebody is calling it.  I kind of doubt that
anyone is, given the small usage in core, but maybe.

            regards, tom lane

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index e70243a..5ff7824 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3217,7 +3217,7 @@ isOtherTempNamespace(Oid namespaceId)
 }

 /*
- * isTempNamespaceInUse - is the given namespace owned and actively used
+ * checkTempNamespaceStatus - is the given namespace owned and actively used
  * by a backend?
  *
  * Note: this can be used while scanning relations in pg_class to detect
@@ -3225,8 +3225,8 @@ isOtherTempNamespace(Oid namespaceId)
  * given database.  The result may be out of date quickly, so the caller
  * must be careful how to handle this information.
  */
-bool
-isTempNamespaceInUse(Oid namespaceId)
+TempNamespaceStatus
+checkTempNamespaceStatus(Oid namespaceId)
 {
     PGPROC       *proc;
     int            backendId;
@@ -3235,25 +3235,25 @@ isTempNamespaceInUse(Oid namespaceId)

     backendId = GetTempNamespaceBackendId(namespaceId);

-    /* No such temporary namespace? */
+    /* No such namespace, or its name shows it's not temp? */
     if (backendId == InvalidBackendId)
-        return false;
+        return TEMP_NAMESPACE_NOT_TEMP;

     /* Is the backend alive? */
     proc = BackendIdGetProc(backendId);
     if (proc == NULL)
-        return false;
+        return TEMP_NAMESPACE_IDLE;

     /* Is the backend connected to the same database we are looking at? */
     if (proc->databaseId != MyDatabaseId)
-        return false;
+        return TEMP_NAMESPACE_IDLE;

     /* Does the backend own the temporary namespace? */
     if (proc->tempNamespaceId != namespaceId)
-        return false;
+        return TEMP_NAMESPACE_IDLE;

-    /* all good to go */
-    return true;
+    /* Yup, so namespace is busy */
+    return TEMP_NAMESPACE_IN_USE;
 }

 /*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6d1f28c..5314228 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2073,7 +2073,7 @@ do_autovacuum(void)
              * We just ignore it if the owning backend is still active and
              * using the temporary schema.
              */
-            if (!isTempNamespaceInUse(classForm->relnamespace))
+            if (checkTempNamespaceStatus(classForm->relnamespace) == TEMP_NAMESPACE_IDLE)
             {
                 /*
                  * The table seems to be orphaned -- although it might be that
@@ -2243,7 +2243,7 @@ do_autovacuum(void)
             continue;
         }

-        if (isTempNamespaceInUse(classForm->relnamespace))
+        if (checkTempNamespaceStatus(classForm->relnamespace) != TEMP_NAMESPACE_IDLE)
         {
             UnlockRelationOid(relid, AccessExclusiveLock);
             continue;
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index d67f93a..3e3a192 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -38,6 +38,16 @@ typedef struct _FuncCandidateList
 }           *FuncCandidateList;

 /*
+ * Result of checkTempNamespaceStatus
+ */
+typedef enum TempNamespaceStatus
+{
+    TEMP_NAMESPACE_NOT_TEMP,    /* nonexistent, or non-temp namespace */
+    TEMP_NAMESPACE_IDLE,        /* exists, belongs to no active session */
+    TEMP_NAMESPACE_IN_USE        /* belongs to some active session */
+} TempNamespaceStatus;
+
+/*
  *    Structure for xxxOverrideSearchPath functions
  */
 typedef struct OverrideSearchPath
@@ -138,7 +148,7 @@ extern bool isTempToastNamespace(Oid namespaceId);
 extern bool isTempOrTempToastNamespace(Oid namespaceId);
 extern bool isAnyTempNamespace(Oid namespaceId);
 extern bool isOtherTempNamespace(Oid namespaceId);
-extern bool isTempNamespaceInUse(Oid namespaceId);
+extern TempNamespaceStatus checkTempNamespaceStatus(Oid namespaceId);
 extern int    GetTempNamespaceBackendId(Oid namespaceId);
 extern Oid    GetTempToastNamespace(void);
 extern void GetTempNamespaceState(Oid *tempNamespaceId,

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: HAVE_WORKING_LINK still needed?
Next
From: Tom Lane
Date:
Subject: Re: Allowing ALTER TYPE to change storage strategy