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

On Wed, 25 Dec 2019 at 07:52, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Tue, Dec 24, 2019 at 04:50:58PM +0530, Mahendra Singh wrote:
> > We can fix this problem by either one way 1) reset myTempNamespace to
> > invalid while drooping schema of temp table 2) should not allow to drop
> > temporary table schema
>
> (Please note that it is better not to cross-post on multiple lists, so

Sorry. I was not aware of multiple mail ids. I will take care in future mails.

> I have removed pgsql-bugs from CC.)

Thanks.

> There is a little bit more to that, as we would basically need to do
> the work of RemoveTempRelationsCallback() once the temp schema is
> dropped, callback registered when the schema is correctly created at
> transaction commit (also we need to make sure that
> RemoveTempRelationsCallback is not called or unregistered if we were
> to authorize DROP SCHEMA on a temp schema). And then all the reset
> done at the beginning of AtEOXact_Namespace() would need to happen.
>

Thanks for quick detailed analysis.

> Anyway, as dropping a temporary schema leads to an inconsistent
> behavior when recreating new temporary objects in a session that
> dropped it, that nobody has actually complained on the matter, and
> that in concept a temporary schema is linked to the session that
> created it, I think that we have a lot of arguments to just forbid the
> operation from happening. Please note as well that it is possible to
> drop temporary schemas of other sessions, still this is limited to
> owners of the schema.

Yes, you are right that we can drop temporary schema of other sessions.

Even after applying your attached patch, I am getting same assert
failure because I am able to drop " temporary schema" from other
session so I think, we should not allow to drop any temporary schema
from any session.

> In short, let's tighten the logic, and we had better back-patch this
> one all the way down, 9.4 being broken. Attached is a patch to do

Yes, I also verified that we have to back-patch till v9.4.

> that. The error message generated depends on the state of the session
> so I have not added a test for this reason, and the check is added
> before the ACL check. We could make the error message more generic,
> like "cannot drop temporary namespace". Any thoughts?

I think, we can make error message as "cannot drop temporary schema"

While applying attached patch on HEAD, I got below warnings:

[mahendra(at)localhost postgres]$ git apply drop-temp-schema-v1.patch
drop-temp-schema-v1.patch:9: trailing whitespace.
/*
drop-temp-schema-v1.patch:10: trailing whitespace.
* Prevent drop of a temporary schema as this would mess up with
drop-temp-schema-v1.patch:11: trailing whitespace.
* the end-of-session callback cleaning up all temporary objects.
drop-temp-schema-v1.patch:12: trailing whitespace.
* As the in-memory state is not cleaned up either here, upon
drop-temp-schema-v1.patch:13: trailing whitespace.
* recreation of a temporary schema within the same session the
error: patch failed: src/backend/commands/dropcmds.c:101
error: src/backend/commands/dropcmds.c: patch does not apply

I think, above warnings are due to "trailing CRs" in patch.

Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com