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