Skip to content

Commit fe2ef42

Browse files
tglsfdctglsfdc
committed
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 debcec7 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.
1 parent d93c01f commit fe2ef42

File tree

7 files changed

+64
-24
lines changed

7 files changed

+64
-24
lines changed

‎src/backend/catalog/toasting.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
198198
* Toast tables for regular relations go in pg_toast; those for temp
199199
* relations go into the per-backend temp-toast-table namespace.
200200
*/
201-
if (RelationUsesTempNamespace(rel))
201+
if (isTempOrToastNamespace(rel->rd_rel->relnamespace))
202202
namespaceid = GetTempToastNamespace();
203203
else
204204
namespaceid = PG_TOAST_NAMESPACE;

‎src/backend/commands/copy.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
800800
Assert(rel);
801801

802802
/* check read-only transaction */
803-
if (XactReadOnly && rel->rd_backend != MyBackendId)
803+
if (XactReadOnly && !rel->rd_islocaltemp)
804804
PreventCommandIfReadOnly("COPY FROM");
805805

806806
cstate = BeginCopyFrom(rel, stmt->filename,

‎src/backend/commands/sequence.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ nextval_internal(Oid relid)
552552
RelationGetRelationName(seqrel))));
553553

554554
/* read-only transactions may only modify temp sequences */
555-
if (seqrel->rd_backend != MyBackendId)
555+
if (!seqrel->rd_islocaltemp)
556556
PreventCommandIfReadOnly("nextval()");
557557

558558
if (elm->last != elm->cached) /* some numbers were cached */
@@ -845,7 +845,7 @@ do_setval(Oid relid, int64 next, bool iscalled)
845845
RelationGetRelationName(seqrel))));
846846

847847
/* read-only transactions may only modify temp sequences */
848-
if (seqrel->rd_backend != MyBackendId)
848+
if (!seqrel->rd_islocaltemp)
849849
PreventCommandIfReadOnly("setval()");
850850

851851
/* lock page' buffer and read tuple */

‎src/backend/commands/tablecmds.c

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,13 +1450,20 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
14501450
errmsg("inherited relation \"%s\" is not a table",
14511451
parent->relname)));
14521452
/* Permanent rels cannot inherit from temporary ones */
1453-
if (relpersistence != RELPERSISTENCE_TEMP
1454-
&& RelationUsesTempNamespace(relation))
1453+
if (relpersistence != RELPERSISTENCE_TEMP &&
1454+
relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
14551455
ereport(ERROR,
14561456
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
14571457
errmsg("cannot inherit from temporary relation \"%s\"",
14581458
parent->relname)));
14591459

1460+
/* If existing rel is temp, it must belong to this session */
1461+
if (relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
1462+
!relation->rd_islocaltemp)
1463+
ereport(ERROR,
1464+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
1465+
errmsg("cannot inherit from temporary relation of another session")));
1466+
14601467
/*
14611468
* We should have an UNDER permission flag for this, but for now,
14621469
* demand that creator of a child table own the parent.
@@ -5767,6 +5774,10 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
57675774
ereport(ERROR,
57685775
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
57695776
errmsg("constraints on temporary tables may reference only temporary tables")));
5777+
if (!pkrel->rd_islocaltemp || !rel->rd_islocaltemp)
5778+
ereport(ERROR,
5779+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
5780+
errmsg("constraints on temporary tables must involve temporary tables of this session")));
57705781
break;
57715782
}
57725783

@@ -8905,13 +8916,27 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
89058916
ATSimplePermissions(parent_rel, ATT_TABLE);
89068917

89078918
/* Permanent rels cannot inherit from temporary ones */
8908-
if (RelationUsesTempNamespace(parent_rel)
8909-
&& !RelationUsesTempNamespace(child_rel))
8919+
if (parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
8920+
child_rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
89108921
ereport(ERROR,
89118922
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
89128923
errmsg("cannot inherit from temporary relation \"%s\"",
89138924
RelationGetRelationName(parent_rel))));
89148925

8926+
/* If parent rel is temp, it must belong to this session */
8927+
if (parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
8928+
!parent_rel->rd_islocaltemp)
8929+
ereport(ERROR,
8930+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
8931+
errmsg("cannot inherit from temporary relation of another session")));
8932+
8933+
/* Ditto for the child */
8934+
if (child_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
8935+
!child_rel->rd_islocaltemp)
8936+
ereport(ERROR,
8937+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
8938+
errmsg("cannot inherit to temporary relation of another session")));
8939+
89158940
/*
89168941
* Check for duplicates in the list of parents, and determine the highest
89178942
* inhseqno already present; we'll use the next one for the new parent.

‎src/backend/utils/adt/dbsize.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ pg_tablespace_size_name(PG_FUNCTION_ARGS)
259259

260260
/*
261261
* calculate size of (one fork of) a relation
262+
*
263+
* Note: we can safely apply this to temp tables of other sessions, so there
264+
* is no check here or at the call sites for that.
262265
*/
263266
static int64
264267
calculate_relation_size(RelFileNode *rfn, BackendId backend, ForkNumber forknum)
@@ -313,7 +316,7 @@ pg_relation_size(PG_FUNCTION_ARGS)
313316
* that makes queries like "SELECT pg_relation_size(oid) FROM pg_class"
314317
* less robust, because while we scan pg_class with an MVCC snapshot,
315318
* someone else might drop the table. It's better to return NULL for
316-
* alread-dropped tables than throw an error and abort the whole query.
319+
* already-dropped tables than throw an error and abort the whole query.
317320
*/
318321
if (rel == NULL)
319322
PG_RETURN_NULL();

‎src/backend/utils/cache/relcache.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -852,20 +852,33 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
852852
case RELPERSISTENCE_UNLOGGED:
853853
case RELPERSISTENCE_PERMANENT:
854854
relation->rd_backend = InvalidBackendId;
855+
relation->rd_islocaltemp = false;
855856
break;
856857
case RELPERSISTENCE_TEMP:
857858
if (isTempOrToastNamespace(relation->rd_rel->relnamespace))
859+
{
858860
relation->rd_backend = MyBackendId;
861+
relation->rd_islocaltemp = true;
862+
}
859863
else
860864
{
861865
/*
862-
* If it's a local temp table, but not one of ours, we have to
863-
* use the slow, grotty method to figure out the owning
864-
* backend.
866+
* If it's a temp table, but not one of ours, we have to use
867+
* the slow, grotty method to figure out the owning backend.
868+
*
869+
* Note: it's possible that rd_backend gets set to MyBackendId
870+
* here, in case we are looking at a pg_class entry left over
871+
* from a crashed backend that coincidentally had the same
872+
* BackendId we're using. We should *not* consider such a
873+
* table to be "ours"; this is why we need the separate
874+
* rd_islocaltemp flag. The pg_class entry will get flushed
875+
* if/when we clean out the corresponding temp table namespace
876+
* in preparation for using it.
865877
*/
866878
relation->rd_backend =
867879
GetTempNamespaceBackendId(relation->rd_rel->relnamespace);
868880
Assert(relation->rd_backend != InvalidBackendId);
881+
relation->rd_islocaltemp = false;
869882
}
870883
break;
871884
default:
@@ -1386,6 +1399,7 @@ formrdesc(const char *relationName, Oid relationReltype,
13861399
relation->rd_createSubid = InvalidSubTransactionId;
13871400
relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
13881401
relation->rd_backend = InvalidBackendId;
1402+
relation->rd_islocaltemp = false;
13891403

13901404
/*
13911405
* initialize relation tuple form
@@ -2535,16 +2549,19 @@ RelationBuildLocalRelation(const char *relname,
25352549
/* needed when bootstrapping: */
25362550
rel->rd_rel->relowner = BOOTSTRAP_SUPERUSERID;
25372551

2538-
/* set up persistence; rd_backend is a function of persistence type */
2552+
/* set up persistence and relcache fields dependent on it */
25392553
rel->rd_rel->relpersistence = relpersistence;
25402554
switch (relpersistence)
25412555
{
25422556
case RELPERSISTENCE_UNLOGGED:
25432557
case RELPERSISTENCE_PERMANENT:
25442558
rel->rd_backend = InvalidBackendId;
2559+
rel->rd_islocaltemp = false;
25452560
break;
25462561
case RELPERSISTENCE_TEMP:
2562+
Assert(isTempOrToastNamespace(relnamespace));
25472563
rel->rd_backend = MyBackendId;
2564+
rel->rd_islocaltemp = true;
25482565
break;
25492566
default:
25502567
elog(ERROR, "invalid relpersistence: %c", relpersistence);

‎src/include/utils/rel.h

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ typedef struct RelationData
8585
bool rd_isvalid; /* relcache entry is valid */
8686
char rd_indexvalid; /* state of rd_indexlist: 0 = not valid, 1 =
8787
* valid, 2 = temporarily forced */
88+
bool rd_islocaltemp; /* rel is a temp rel of this session */
8889

8990
/*
9091
* rd_createSubid is the ID of the highest subtransaction the rel has
@@ -362,22 +363,16 @@ typedef struct StdRdOptions
362363
#define RelationUsesLocalBuffers(relation) \
363364
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
364365

365-
/*
366-
* RelationUsesTempNamespace
367-
* True if relation's catalog entries live in a private namespace.
368-
*/
369-
#define RelationUsesTempNamespace(relation) \
370-
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
371-
372366
/*
373367
* RELATION_IS_LOCAL
374368
* If a rel is either temp or newly created in the current transaction,
375-
* it can be assumed to be visible only to the current backend.
369+
* it can be assumed to be accessible only to the current backend.
370+
* This is typically used to decide that we can skip acquiring locks.
376371
*
377372
* Beware of multiple eval of argument
378373
*/
379374
#define RELATION_IS_LOCAL(relation) \
380-
((relation)->rd_backend == MyBackendId || \
375+
((relation)->rd_islocaltemp || \
381376
(relation)->rd_createSubid != InvalidSubTransactionId)
382377

383378
/*
@@ -387,8 +382,8 @@ typedef struct StdRdOptions
387382
* Beware of multiple eval of argument
388383
*/
389384
#define RELATION_IS_OTHER_TEMP(relation) \
390-
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP \
391-
&& (relation)->rd_backend != MyBackendId)
385+
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
386+
!(relation)->rd_islocaltemp)
392387

393388
/* routines in utils/cache/relcache.c */
394389
extern void RelationIncrementReferenceCount(Relation rel);

0 commit comments

Comments
 (0)