Fix failure to ignore leftover temp tables after a server crash.

During crash recovery, we remove disk files belonging to temporary tables,
but the system catalog entries for such tables are intentionally not
cleaned up right away.  Instead, the first backend that uses a temp schema
is expected to clean out any leftover objects therein.  This approach
requires that we be careful to ignore leftover temp tables (since any
actual access attempt would fail), *even if their BackendId matches our
session*, if we have not yet established use of the session's corresponding
temp schema.  That worked fine in the past, but was broken by commit
debcec7dc31a992703911a9953e299c8d730c778 which incorrectly removed the
rd_islocaltemp relcache flag.  Put it back, and undo various changes
that substituted tests like "rel->rd_backend == MyBackendId" for use
of a state-aware flag.  Per trouble report from Heikki Linnakangas.

Back-patch to 9.1 where the erroneous change was made.  In the back
branches, be careful to add rd_islocaltemp in a spot in the struct that
was alignment padding before, so as not to break existing add-on code.

diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 2979819e966b2aed45f29a33be413962df18fcf9..4424260605b164f0a8b31c0b9e7c1216ae3ec12a 100644 (file)
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -196,7 +196,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
         * Toast tables for regular relations go in pg_toast; those for temp
         * relations go into the per-backend temp-toast-table namespace.
         */
-       if (RelationUsesTempNamespace(rel))
+       if (isTempOrToastNamespace(rel->rd_rel->relnamespace))
                namespaceid = GetTempToastNamespace();
        else
                namespaceid = PG_TOAST_NAMESPACE;

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 8200d98139933a0f1cb4a15a239bdf86a03984e2..4172c0cfbe68a8a9c6c960d1b40d1865766ada10 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -806,7 +806,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
                Assert(rel);
 
                /* check read-only transaction */
-               if (XactReadOnly && rel->rd_backend != MyBackendId)
+               if (XactReadOnly && !rel->rd_islocaltemp)
                        PreventCommandIfReadOnly("COPY FROM");
 
                cstate = BeginCopyFrom(rel, stmt->filename,

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 634ce3f7188933eb354468a2e5f337cfa57cbd57..5583e721ead8f51d9b8627e1ee73200e2abd7eee 100644 (file)
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -553,7 +553,7 @@ nextval_internal(Oid relid)
                                                RelationGetRelationName(seqrel))));
 
        /* read-only transactions may only modify temp sequences */
-       if (seqrel->rd_backend != MyBackendId)
+       if (!seqrel->rd_islocaltemp)
                PreventCommandIfReadOnly("nextval()");
 
        if (elm->last != elm->cached)           /* some numbers were cached */
@@ -846,7 +846,7 @@ do_setval(Oid relid, int64 next, bool iscalled)
                                                RelationGetRelationName(seqrel))));
 
        /* read-only transactions may only modify temp sequences */
-       if (seqrel->rd_backend != MyBackendId)
+       if (!seqrel->rd_islocaltemp)
                PreventCommandIfReadOnly("setval()");
 
        /* lock page' buffer and read tuple */

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4065740b336079305bc663b57ef8ae96b9692100..761374741db9f7c329f845e9952e4daed05f288d 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1451,13 +1451,20 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
                                         errmsg("inherited relation \"%s\" is not a table",
                                                        parent->relname)));
                /* Permanent rels cannot inherit from temporary ones */
-               if (relpersistence != RELPERSISTENCE_TEMP
-                       && RelationUsesTempNamespace(relation))
+               if (relpersistence != RELPERSISTENCE_TEMP &&
+                       relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                         errmsg("cannot inherit from temporary relation \"%s\"",
                                                        parent->relname)));
 
+               /* If existing rel is temp, it must belong to this session */
+               if (relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+                       !relation->rd_islocaltemp)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                        errmsg("cannot inherit from temporary relation of another session")));
+
                /*
                 * We should have an UNDER permission flag for this, but for now,
                 * demand that creator of a child table own the parent.
@@ -5768,6 +5775,10 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
                                ereport(ERROR,
                                                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                                                 errmsg("constraints on temporary tables may reference only temporary tables")));
+                       if (!pkrel->rd_islocaltemp || !rel->rd_islocaltemp)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+                                                errmsg("constraints on temporary tables must involve temporary tables of this session")));
                        break;
        }
 
@@ -8884,13 +8895,27 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
        ATSimplePermissions(parent_rel, ATT_TABLE);
 
        /* Permanent rels cannot inherit from temporary ones */
-       if (RelationUsesTempNamespace(parent_rel)
-               && !RelationUsesTempNamespace(child_rel))
+       if (parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+               child_rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
                ereport(ERROR,
                                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                 errmsg("cannot inherit from temporary relation \"%s\"",
                                                RelationGetRelationName(parent_rel))));
 
+       /* If parent rel is temp, it must belong to this session */
+       if (parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+               !parent_rel->rd_islocaltemp)
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("cannot inherit from temporary relation of another session")));
+
+       /* Ditto for the child */
+       if (child_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+               !child_rel->rd_islocaltemp)
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("cannot inherit to temporary relation of another session")));
+
        /*
         * Check for duplicates in the list of parents, and determine the highest
         * inhseqno already present; we'll use the next one for the new parent.

diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index cd23334a1a5118269f96428c54c27bdef1db7012..581fbfd7fa764bba9c794c54eb54dcec87f531dd 100644 (file)
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -260,6 +260,9 @@ pg_tablespace_size_name(PG_FUNCTION_ARGS)
 
 /*
  * calculate size of (one fork of) a relation
+ *
+ * Note: we can safely apply this to temp tables of other sessions, so there
+ * is no check here or at the call sites for that.
  */
 static int64
 calculate_relation_size(RelFileNode *rfn, BackendId backend, ForkNumber forknum)
@@ -314,7 +317,7 @@ pg_relation_size(PG_FUNCTION_ARGS)
         * that makes queries like "SELECT pg_relation_size(oid) FROM pg_class"
         * less robust, because while we scan pg_class with an MVCC snapshot,
         * someone else might drop the table. It's better to return NULL for
-        * alread-dropped tables than throw an error and abort the whole query.
+        * already-dropped tables than throw an error and abort the whole query.
         */
        if (rel == NULL)
                PG_RETURN_NULL();

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 9a504f80251b6c1c548672282612ec130ba70401..f2a9c5dffb24084001d9cd3d05eca21c638d420e 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -853,20 +853,33 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
                case RELPERSISTENCE_UNLOGGED:
                case RELPERSISTENCE_PERMANENT:
                        relation->rd_backend = InvalidBackendId;
+                       relation->rd_islocaltemp = false;
                        break;
                case RELPERSISTENCE_TEMP:
                        if (isTempOrToastNamespace(relation->rd_rel->relnamespace))
+                       {
                                relation->rd_backend = MyBackendId;
+                               relation->rd_islocaltemp = true;
+                       }
                        else
                        {
                                /*
-                                * If it's a local temp table, but not one of ours, we have to
-                                * use the slow, grotty method to figure out the owning
-                                * backend.
+                                * If it's a temp table, but not one of ours, we have to use
+                                * the slow, grotty method to figure out the owning backend.
+                                *
+                                * Note: it's possible that rd_backend gets set to MyBackendId
+                                * here, in case we are looking at a pg_class entry left over
+                                * from a crashed backend that coincidentally had the same
+                                * BackendId we're using.  We should *not* consider such a
+                                * table to be "ours"; this is why we need the separate
+                                * rd_islocaltemp flag.  The pg_class entry will get flushed
+                                * if/when we clean out the corresponding temp table namespace
+                                * in preparation for using it.
                                 */
                                relation->rd_backend =
                                        GetTempNamespaceBackendId(relation->rd_rel->relnamespace);
                                Assert(relation->rd_backend != InvalidBackendId);
+                               relation->rd_islocaltemp = false;
                        }
                        break;
                default:
@@ -1387,6 +1400,7 @@ formrdesc(const char *relationName, Oid relationReltype,
        relation->rd_createSubid = InvalidSubTransactionId;
        relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
        relation->rd_backend = InvalidBackendId;
+       relation->rd_islocaltemp = false;
 
        /*
         * initialize relation tuple form
@@ -2538,16 +2552,19 @@ RelationBuildLocalRelation(const char *relname,
        /* needed when bootstrapping: */
        rel->rd_rel->relowner = BOOTSTRAP_SUPERUSERID;
 
-       /* set up persistence; rd_backend is a function of persistence type */
+       /* set up persistence and relcache fields dependent on it */
        rel->rd_rel->relpersistence = relpersistence;
        switch (relpersistence)
        {
                case RELPERSISTENCE_UNLOGGED:
                case RELPERSISTENCE_PERMANENT:
                        rel->rd_backend = InvalidBackendId;
+                       rel->rd_islocaltemp = false;
                        break;
                case RELPERSISTENCE_TEMP:
+                       Assert(isTempOrToastNamespace(relnamespace));
                        rel->rd_backend = MyBackendId;
+                       rel->rd_islocaltemp = true;
                        break;
                default:
                        elog(ERROR, "invalid relpersistence: %c", relpersistence);

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4669d8a67ef4298c591accbc81710c76e1efa535..f7f84e6f71e8e4c36e66dbb3b5b8614b846a0bae 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -81,6 +81,7 @@ typedef struct RelationData
        struct SMgrRelationData *rd_smgr;       /* cached file handle, or NULL */
        int                     rd_refcnt;              /* reference count */
        BackendId       rd_backend;             /* owning backend id, if temporary relation */
+       bool            rd_islocaltemp; /* rel is a temp rel of this session */
        bool            rd_isnailed;    /* rel is nailed in cache */
        bool            rd_isvalid;             /* relcache entry is valid */
        char            rd_indexvalid;  /* state of rd_indexlist: 0 = not valid, 1 =
@@ -362,22 +363,16 @@ typedef struct StdRdOptions
 #define RelationUsesLocalBuffers(relation) \
        ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
 
-/*
- * RelationUsesTempNamespace
- *             True if relation's catalog entries live in a private namespace.
- */
-#define RelationUsesTempNamespace(relation) \
-       ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
-
 /*
  * RELATION_IS_LOCAL
  *             If a rel is either temp or newly created in the current transaction,
- *             it can be assumed to be visible only to the current backend.
+ *             it can be assumed to be accessible only to the current backend.
+ *             This is typically used to decide that we can skip acquiring locks.
  *
  * Beware of multiple eval of argument
  */
 #define RELATION_IS_LOCAL(relation) \
-       ((relation)->rd_backend == MyBackendId || \
+       ((relation)->rd_islocaltemp || \
         (relation)->rd_createSubid != InvalidSubTransactionId)
 
 /*
@@ -387,8 +382,8 @@ typedef struct StdRdOptions
  * Beware of multiple eval of argument
  */
 #define RELATION_IS_OTHER_TEMP(relation) \
-       ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP \
-       && (relation)->rd_backend != MyBackendId)
+       ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
+        !(relation)->rd_islocaltemp)
 
 /* routines in utils/cache/relcache.c */
 extern void RelationIncrementReferenceCount(Relation rel);