Re: Temp schema drop leaves an inconsistent state behind

Hi Marko,

Marko Grujic <marko(dot)grujic(at)enterprisedb(dot)com> wrote:
> The reason it wasn't re-created is because the internal static variable
> (myTempNamespace) is left holding the 16409 value internally.

This came up in 2019-2020 [1]. a052f6cbb84 forbade DROP SCHEMA on temp
schemas and was reverted (65192e02441) after Robert and Tom argued that
a superuser should be able to do it, and that code should cope with a
missing schema instead of crashing; 80d76be51cf then fixed autovacuum
that way. Michael pointed out the stale myTempNamespace in that thread,
but it was left as is, so your patches fill that gap in the direction
agreed there. (Cc'ing Michael for that reason.)

I tested them on master d39fda1cc40, with --enable-cassert, and master
without them as the control. Both reported problems are fixed, and 0002
covers more than tables. After dropping the own temp schema, on master
CREATE DOMAIN, CREATE FUNCTION and CREATE STATISTICS in pg_temp fail
with "referenced schema was concurrently dropped", while a composite
type and a temp table with a serial column and an index are created in
the missing schema: 9 rows in pg_class and pg_type point to it. With
0002, everything is created and nothing is left dangling. The same when another superuser session drops
the schema: on master that session leaves a pg_class row with a missing
schema behind when it exits; with 0002, it doesn't.

About 0001: the strlist_to_textarray() part is worth a line in the
commit message, because on master a NULL name is silently dropped from
the array, and that is worse than it looks. With a table whose
pg_namespace row is gone (I deleted it with allow_system_table_mods) and
another table of the same name in public,

pg_get_object_address('table',
(pg_identify_object_as_address('pg_class'::regclass, oid, 0)).object_names,
'{}')

resolves to public.x on master. With 0001 the names are {NULL,x} and it
fails with "name or argument lists may not contain nulls". On master,
pg_identify_object() on that table crashes too, as in your report.

One problem with 0002: if the DROP SCHEMA is rolled back, the session
loses its temp schema, and the next temp table empties it.

CREATE TEMP TABLE keep(a int);
INSERT INTO keep VALUES (1), (2), (3);
BEGIN;
DROP SCHEMA pg_temp_0 CASCADE;
CREATE TEMP TABLE t3(a int);
ROLLBACK;
SELECT count(*) FROM keep;
ERROR: relation "keep" does not exist

keep is still in pg_class, in pg_temp_0, but pg_my_temp_schema() is 0,
pg_temp is gone from the search path, and "SELECT * FROM pg_temp_0.keep"
fails with "cannot access temporary tables of other sessions". The next
CREATE TEMP TABLE goes through InitTempTableNamespace(), finds pg_temp_0,
takes it for a crashed backend's leftover, and RemoveTempRelations()
drops keep. It's the same with ROLLBACK TO SAVEPOINT. On master, keep
still has its 3 rows in both cases.

Nothing undoes the reset in AccessTempTableNamespace() on abort:
AtEOXact_Namespace() sees the myTempNamespaceSubID set by the new
InitTempTableNamespace() and forgets the new schema, but nothing brings
back the old one. The attached diff, on top of 0002, remembers the
forgotten schema and the subtransaction that forgot it, and puts it back
if that subtransaction aborts, unless the schema was created in it; on
subtransaction commit it moves to the parent, like myTempNamespaceSubID.
If the drop was committed earlier, what it puts back is still stale, and
the next access forgets it again. The diff adds both rollback cases to
your test in temp.sql, which fails with 0002 alone.

With it, those two cases behave as on master, and so do these:

- a temp schema created, dropped and rolled back in one transaction
- a committed drop, then an aborted CREATE TEMP TABLE, then another
- a drop undone by ROLLBACK TO an outer savepoint, with a savepoint
released in between
- a drop released from a savepoint, then the whole transaction rolled
back

0002 alone loses keep in the last two. make check (239) and isolation
(133) pass with 0001 and 0002, and with the diff on top; pgindent leaves
it unchanged.

The attached temp_schema_drop_cases.sql has all of the above as a plain
psql script, one case per session, with what each should return. Case 8
deletes a pg_namespace row, so it is meant for a scratch database. On
master cases 1, 5 and 8 fail (8 by crashing, as the last statement), with
0001 and 0002 cases 2, 3, 6 and 7, and with the diff none.

[1] https://www.postgresql.org/message-id/CAKYtNAr9Zq%3D1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg%40mail.gmail.com

Regards,
Manu