Re: Temp schema drop leaves an inconsistent state behind

Hi Jim,

> On top of that, a just dropped schema can still be used to create
> temporary objects, which creates a pg_class orphan:
...
> IIUC the problem is that activeCreationNamespace is used when the
> relpersistence is not RELPERSISTENCE_TEMP. So with pg_temp first in
> search_path, activeCreationNamespace is the dropped schema.

That's right, and 0002 does not cover it, nor does my diff on top of
it. I ran each kind of CREATE as the first statement after the drop,
with pg_temp first in search_path and no TEMP keyword, one fresh
cluster per statement, on master and with 0001+0002 (the rollback diff
gives the same results):

CREATE TABLE, CREATE VIEW, CREATE TYPE (composite)
master: orphaned; 0002: fine
CREATE SEQUENCE, CREATE TABLE AS
master: orphaned; 0002: orphaned
CREATE TYPE (enum), CREATE DOMAIN, CREATE FUNCTION
master and 0002: "referenced schema was concurrently dropped"
CREATE MATERIALIZED VIEW
master: orphaned; 0002: assertion failure

TRAP: failed Assert("relation->rd_backend != INVALID_PROC_NUMBER"),
File: "relcache.c", Line: 1189

reached from RefreshMatViewByOid() -> finish_heap_swap() ->
reindex_relation() -> relcache invalidation. Without assertions
the command succeeds, but the view is then unreachable ("relation
"x" does not exist") and orphaned. On master it is at least
usable.

With the pg_temp. qualification all of them work with 0002, which is
what I tested in my previous mail.

Why: 0002 resets myTempNamespace in AccessTempTableNamespace(), and
RangeVarGetCreationNamespace() only calls that when the relation is
already known to be temporary. Otherwise it takes
activeCreationNamespace, the dropped schema, and
RangeVarAdjustRelationPersistence() marks the relation temporary
afterwards. CREATE TABLE and CREATE VIEW look the namespace up twice,
the second time in DefineRelation(), when the relation is already
temporary, so they reach the reset. CREATE SEQUENCE looks it up only
in DefineRelation(). That gives a check: CREATE SEQUENCE IF NOT
EXISTS, which adds a lookup before DefineRelation(), comes out right
with 0002, and plain CREATE SEQUENCE does not. For the materialized
view the reset happens in the middle of the command: the view is
created in the dropped schema, and the REFRESH that fills it creates
the new heap through AccessTempTableNamespace(), which moves the
session to a new temp schema underneath it.

The attached diff, on top of 0002, checks in
preprocessNamespacePath() that myTempNamespace still exists before
putting it in the search path. If it doesn't, pg_temp is treated as
not created yet, as in a session that has not used it, so creating
through the search path goes through AccessTempTableNamespace(true)
and the reset in 0002. With it:

- all sixteen statements in the test (the ones above, CREATE SEQUENCE
IF NOT EXISTS, and five with the pg_temp. qualification) create the
object in a new temp schema, with no orphans and no assertion
failure, both with 0001+0002 alone and with the rollback diff;
- your case leaves no orphan, and pg_dump succeeds;
- the cases from my previous mail give the same output as before, and
so do their variants creating through the search path: after a
rolled-back drop, keep still has its rows;
- make check (239) and isolation (133) pass, with no new warnings.

The extra syscache lookup only runs when the search path is
recomputed. I did not add a regression test; your sequence case would
be the natural one next to the others in temp.sql.

The scripts and all the outputs are attached.

Regards,
Manu