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,