From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Mahendra Singh <mahi6run(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | 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 |
Date: | 2020-01-06 17:25:19 |
Message-ID: | CA+TgmobPJ+CQGr1nKnt4rNikgBqHvXtJ+x8UW0U_kR6ksf2D-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Sun, Jan 5, 2020 at 8:42 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> The behavior of the code in 246a6c8 has changed so as a non-existing
> temporary namespace is considered as not in use, in which case
> autovacuum would consider this relation to be orphaned, and it would
> then try to drop it. Anyway, just a revert of the patch is not a good
> idea either, because keeping around the old behavior allows any user
> to create orphaned relations that autovacuum would just ignore in
> 9.4~10, leading to ultimately a forced shutdown of the instance as no
> cleanup can happen if this goes unnoticed. This also puts pg_class
> into an inconsistent state as pg_class entries would include
> references to a namespace that does not exist for sessions still
> holding its own references to myTempNamespace/myTempToastNamespace.
I'm not arguing for a revert of 246a6c8. I think we should just change this:
ereport(LOG,
(errmsg("autovacuum: dropping orphan
temp table \"%s.%s.%s\"",
get_database_name(MyDatabaseId),
get_namespace_name(classForm->relnamespace),
NameStr(classForm->relname))));
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. As a side
benefit, this would also provide some insurance against other
possibly-problematic situations, like a corrupted pg_class row with a
garbage value in the relnamespace field, which is something I've seen
multiple times in the field.
I can't quite understand your comments about why we shouldn't do that,
but the reported bug is just a null pointer reference. Incredibly,
autovacuum.c seems to have been using get_namespace_name() without a
null check since 2006, so it's not really the fault of your patch as I
had originally thought. I wonder how in the world we've managed to get
away with it for as long as we have.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-01-06 17:33:47 | 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 |
Previous Message | PG Bug reporting form | 2020-01-06 14:58:32 | BUG #16193: Tools menu in pgAdmin is not shown |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-01-06 17:33:47 | 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 |
Previous Message | Tom Lane | 2020-01-06 17:22:50 | Re: proposal: minscale, rtrim, btrim functions for numeric |