Re: isTempNamespaceInUse() is incorrect with its handling of MyBackendId

Julien Rouhaud <rjuju123@gmail.com>

From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-01-13T12:09:01Z
Lists: pgsql-hackers
On Mon, Jan 13, 2020 at 06:37:03PM +0900, Michael Paquier wrote:
> Hi all,
>
> While reviewing some code in namespace.c, I have bumped into the
> following issue introduced by 246a6c8:
> diff --git a/src/backend/catalog/namespace.c
> b/src/backend/catalog/namespace.c
> index c82f9fc4b5..e70243a008 100644
> --- a/src/backend/catalog/namespace.c
> +++ b/src/backend/catalog/namespace.c
> @@ -3235,8 +3235,8 @@ isTempNamespaceInUse(Oid namespaceId)
>
>     backendId = GetTempNamespaceBackendId(namespaceId);
>
> -   if (backendId == InvalidBackendId ||
> -       backendId == MyBackendId)
> +   /* No such temporary namespace? */
> +   if (backendId == InvalidBackendId)
>         return false;
>
> The current logic of isTempNamespaceInUse() would cause a session
> calling the routine to return always false if trying to check if its
> own temporary session is in use, but that's incorrect.

Indeed.

> It is actually
> safe to remove the check on MyBackendId as the code would fall back on
> a check equivalent to MyProc->tempNamespaceId a bit down as per the
> attached, so let's fix it.
>
> Thoughts?

But that means an extraneous call to BackendIdGetProc() in that case, it seems
better to avoid it if we already have the information.



Commits

  1. Fix buggy logic in isTempNamespaceInUse()