On Fri, Dec 27, 2019 at 4:06 AM Michael Paquier <mich...@paquier.xyz> wrote: > And committed down to 9.4. We use much more "temporary schema" in > error messages actually, so I have switched to that.
I think this was a bad idea and that it should be reverted. It seems to me that the problem here is that you introduced a feature which had a bug, namely that it couldn't tolerate concurrency, and when somebody discovered the bug, you "fixed" it not by making the code able to tolerate concurrent activity but by preventing concurrent activity from happening in the first place. I think that's wrong on general principle. In this specific case, DROP SCHEMA on another temporary sessions's schema is a feature which has existed for a very long time and which I have used on multiple occasions to repair damaged databases. Suppose, for example, there's a catalog entry that prevents the schema from being dropped. Before this commit, you could fix it or delete the entry and then retry the drop. Now, you can't. You can maybe wait for autovacuum to retry it or something, assuming autovacuum is working and you're in multi-user mode. But even if that weren't the case, this seems like a very fragile fix. Maybe someday we'll allow multiple autovacuum workers in the same database, and the problem comes back. Maybe some user who can't drop the schema because of this arbitrary prohibition will find themselves forced to delete the pg_namespace row by hand and then crash the server. Most server code is pretty careful that to either tolerate missing system catalog tuples or elog(ERROR), not crash (e.g. cache lookup failed for ...). This code shouldn't be an exception to that rule. Also, as a matter of procedure, 3 days from first post to commit is not a lot, especially when the day something is posted is Christmas Eve. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
