I wrote:
> On the whole I think resurrecting the rd_islocaltemp flag might be the
> best thing. We can keep the use of "relpersistence ==
> RELPERSISTENCE_TEMP" to do what rd_istemp used to do, it's just the
> equation of "rd_backend == MyBackendId" with "is my temp table" that's
> bogus. And this way gets rid of some unnecessary cross-branch coding
> differences.
Here is a draft patch for this. Thoughts/objections?
One thing I noticed that maybe needs more work is that tablecmds.c in
general seems mighty willing to hack upon temp tables belonging to other
sessions. I added tests for that in the places where there already were
checks on relpersistence, but I wonder whether we ought not simply
forbid all forms of ALTER on nonlocal temp rels, right up at the top.
regards, tom lane
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 2979819e966b2aed45f29a33be413962df18fcf9..4424260605b164f0a8b31c0b9e7c1216ae3ec12a 100644
*** a/src/backend/catalog/toasting.c
--- b/src/backend/catalog/toasting.c
*************** create_toast_table(Relation rel, Oid toa
*** 196,202 ****
* 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))
namespaceid = GetTempToastNamespace();
else
namespaceid = PG_TOAST_NAMESPACE;
--- 196,202 ----
* Toast tables for regular relations go in pg_toast; those for temp
* relations go into the per-backend temp-toast-table namespace.
*/
! 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
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*************** DoCopy(const CopyStmt *stmt, const char
*** 806,812 ****
Assert(rel);
/* check read-only transaction */
! if (XactReadOnly && rel->rd_backend != MyBackendId)
PreventCommandIfReadOnly("COPY FROM");
cstate = BeginCopyFrom(rel, stmt->filename,
--- 806,812 ----
Assert(rel);
/* check read-only transaction */
! 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
*** a/src/backend/commands/sequence.c
--- b/src/backend/commands/sequence.c
*************** nextval_internal(Oid relid)
*** 553,559 ****
RelationGetRelationName(seqrel))));
/* read-only transactions may only modify temp sequences */
! if (seqrel->rd_backend != MyBackendId)
PreventCommandIfReadOnly("nextval()");
if (elm->last != elm->cached) /* some numbers were cached */
--- 553,559 ----
RelationGetRelationName(seqrel))));
/* read-only transactions may only modify temp sequences */
! if (!seqrel->rd_islocaltemp)
PreventCommandIfReadOnly("nextval()");
if (elm->last != elm->cached) /* some numbers were cached */
*************** do_setval(Oid relid, int64 next, bool is
*** 846,852 ****
RelationGetRelationName(seqrel))));
/* read-only transactions may only modify temp sequences */
! if (seqrel->rd_backend != MyBackendId)
PreventCommandIfReadOnly("setval()");
/* lock page' buffer and read tuple */
--- 846,852 ----
RelationGetRelationName(seqrel))));
/* read-only transactions may only modify temp sequences */
! 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
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** MergeAttributes(List *schema, List *supe
*** 1451,1463 ****
errmsg("inherited relation \"%s\" is not a table",
parent->relname)));
/* Permanent rels cannot inherit from temporary ones */
! if (relpersistence != RELPERSISTENCE_TEMP
! && RelationUsesTempNamespace(relation))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot inherit from temporary relation \"%s\"",
parent->relname)));
/*
* We should have an UNDER permission flag for this, but for now,
* demand that creator of a child table own the parent.
--- 1451,1470 ----
errmsg("inherited relation \"%s\" is not a table",
parent->relname)));
/* Permanent rels cannot inherit from temporary ones */
! 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.
*************** ATAddForeignKeyConstraint(AlteredTableIn
*** 5768,5773 ****
--- 5775,5784 ----
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;
}
*************** ATExecAddInherit(Relation child_rel, Ran
*** 8884,8896 ****
ATSimplePermissions(parent_rel, ATT_TABLE);
/* Permanent rels cannot inherit from temporary ones */
! if (RelationUsesTempNamespace(parent_rel)
! && !RelationUsesTempNamespace(child_rel))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot inherit from temporary relation \"%s\"",
RelationGetRelationName(parent_rel))));
/*
* 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.
--- 8895,8921 ----
ATSimplePermissions(parent_rel, ATT_TABLE);
/* Permanent rels cannot inherit from temporary ones */
! 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
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
*************** pg_tablespace_size_name(PG_FUNCTION_ARGS
*** 260,265 ****
--- 260,268 ----
/*
* 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)
*************** pg_relation_size(PG_FUNCTION_ARGS)
*** 314,320 ****
* 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.
*/
if (rel == NULL)
PG_RETURN_NULL();
--- 317,323 ----
* 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
! * 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
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
*************** RelationBuildDesc(Oid targetRelId, bool
*** 853,872 ****
case RELPERSISTENCE_UNLOGGED:
case RELPERSISTENCE_PERMANENT:
relation->rd_backend = InvalidBackendId;
break;
case RELPERSISTENCE_TEMP:
if (isTempOrToastNamespace(relation->rd_rel->relnamespace))
relation->rd_backend = MyBackendId;
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.
*/
relation->rd_backend =
GetTempNamespaceBackendId(relation->rd_rel->relnamespace);
Assert(relation->rd_backend != InvalidBackendId);
}
break;
default:
--- 853,885 ----
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 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:
*************** formrdesc(const char *relationName, Oid
*** 1387,1392 ****
--- 1400,1406 ----
relation->rd_createSubid = InvalidSubTransactionId;
relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
relation->rd_backend = InvalidBackendId;
+ relation->rd_islocaltemp = false;
/*
* initialize relation tuple form
*************** RelationBuildLocalRelation(const char *r
*** 2538,2553 ****
/* needed when bootstrapping: */
rel->rd_rel->relowner = BOOTSTRAP_SUPERUSERID;
! /* set up persistence; rd_backend is a function of persistence type */
rel->rd_rel->relpersistence = relpersistence;
switch (relpersistence)
{
case RELPERSISTENCE_UNLOGGED:
case RELPERSISTENCE_PERMANENT:
rel->rd_backend = InvalidBackendId;
break;
case RELPERSISTENCE_TEMP:
rel->rd_backend = MyBackendId;
break;
default:
elog(ERROR, "invalid relpersistence: %c", relpersistence);
--- 2552,2570 ----
/* needed when bootstrapping: */
rel->rd_rel->relowner = BOOTSTRAP_SUPERUSERID;
! /* 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
*** a/src/include/utils/rel.h
--- b/src/include/utils/rel.h
*************** typedef struct RelationData
*** 81,86 ****
--- 81,87 ----
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 =
*************** typedef struct StdRdOptions
*** 363,383 ****
((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.
*
* Beware of multiple eval of argument
*/
#define RELATION_IS_LOCAL(relation) \
! ((relation)->rd_backend == MyBackendId || \
(relation)->rd_createSubid != InvalidSubTransactionId)
/*
--- 364,378 ----
((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 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_islocaltemp || \
(relation)->rd_createSubid != InvalidSubTransactionId)
/*
*************** typedef struct StdRdOptions
*** 387,394 ****
* Beware of multiple eval of argument
*/
#define RELATION_IS_OTHER_TEMP(relation) \
! ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP \
! && (relation)->rd_backend != MyBackendId)
/* routines in utils/cache/relcache.c */
extern void RelationIncrementReferenceCount(Relation rel);
--- 382,389 ----
* Beware of multiple eval of argument
*/
#define RELATION_IS_OTHER_TEMP(relation) \
! ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
! !(relation)->rd_islocaltemp)
/* routines in utils/cache/relcache.c */
extern void RelationIncrementReferenceCount(Relation rel);
--
Sent via pgsql-bugs mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
