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
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Mahendra Singh <mahi6run@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-01-07T00:22:16Z
Lists: pgsql-bugs, pgsql-hackers
On Mon, Jan 06, 2020 at 12:33:47PM -0500, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I'm not arguing for a revert of 246a6c8. I think we should just change this: >> ... >> To look more like: > >> char *nspname = get_namespace_name(classForm->relnamespace); >> if (nspname != NULL) >> ereport(..."autovacuum: dropping orphan temp table \"%s.%s.%s\"...) >> else >> ereport(..."autovacuum: dropping orphan temp table with OID %u"....) > >> If we do that, then I think we can just revert >> a052f6cbb84e5630d50b68586cecc127e64be639 completely. > > +1 to both of those --- although I think we could still provide the > table name in the null-nspname case. Okay for the first one, printing the OID sounds like a good idea. Like Tom, I would prefer keeping the relation name with "(null)" for the schema name. Or even better, could we just print the OID all the time? What's preventing us from showing that information in the first place? And that still looks good to have when debugging issues IMO for orphaned entries. For the second one, I would really wish that we keep the restriction put in place by a052f6c until we actually figure out how to make the operation safe in the ways we want it to work because this puts the catalogs into an inconsistent state for any object type able to use a temporary schema, like functions, domains etc. for example able to use "pg_temp" as a synonym for the temp namespace name. And any connected user is able to do that. On top of that, except for tables, these could remain as orphaned entries after a crash, no? Allowing the operation only via allow_system_table_mods gives an exit path actually if you really wish to do so, which is fine by me as startup controls that, aka an administrator. In short, I don't think that it is sane to keep in place the property, visibly accidental (?) for any user to create inconsistent catalog entries using a static state in the session which is incorrect in namespace.c, except if we make DROP SCHEMA on a temporary schema have a behavior close to DISCARD TEMP. Again, for the owner of the session that's simple, no clear idea how to do that safely when the drop is done from another session not owning the temp schema. > Maybe we haven't. It's not clear that infrequent autovac crashes would > get reported to us, or that we'd successfully find the cause if they were. > > I think what you propose above is a back-patchable bug fix. Yeah, likely it is safer to fix the logs in the long run down to 9.4. -- Michael
Commits
-
Avoid failure if autovacuum tries to access a just-dropped temp namespace.
- d7684c38a5e9 12.3 landed
- 80d76be51cfb 13.0 landed
- 7ea20a2bc6bd 11.8 landed
-
Revert "Forbid DROP SCHEMA on temporary namespaces"
- b83ba2e6e706 9.4.26 landed
- 86949c2f1207 9.5.21 landed
- 50174b70e779 9.6.17 landed
- 0c046f816902 10.12 landed
- 789bc293b341 11.7 landed
- a1c003e5ff82 12.2 landed
- 65192e02441c 13.0 landed
-
Forbid DROP SCHEMA on temporary namespaces
- 898e0c650097 9.4.26 landed
- 12cb5478a2a4 9.5.21 landed
- cb6f94a5219d 9.6.17 landed
- f1958351eae2 10.12 landed
- 786540085dbd 11.7 landed
- 1dd88201ad6e 12.2 landed
- a052f6cbb84e 13.0 landed
-
Make autovacuum more aggressive to remove orphaned temp tables
- 246a6c8f7b23 12.0 cited
-
Code review for early drop of orphaned temp relations in autovacuum.
- dafa0848da11 10.0 cited