Skip to content

Commit 7ed8ce8

Browse files
committed
Reject modifying a temp table of another session with ALTER TABLE.
Normally this case isn't even reachable by non-superusers, since permissions checks prevent naming such a table. However, it is possible to make it happen by altering a parent table whose child is another session's temp table. We definitely can't support any such ALTER that requires modifying the contents of such a table, since we lack access to the other session's temporary-buffer pool. But there seems no good reason to allow it even if it'd only require changing catalog contents. One reason not to allow it is that we'd rather not expose the implementation-dependent behavior of whether a specific ALTER requires touching the table contents. Another is that there may be (in future, even if not today) optimizations that assume that a session's own temp tables won't be modified by other sessions. Hence, add a RELATION_IS_OTHER_TEMP() check to all the places where ALTER TABLE currently does CheckTableNotInUse(). (I looked through all other callers of CheckTableNotInUse(), and they seem OK already.) Per bug #18492 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/[email protected]
1 parent 2dc1dea commit 7ed8ce8

File tree

1 file changed

+47
-13
lines changed

1 file changed

+47
-13
lines changed

src/backend/commands/tablecmds.c

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ static CoercionPathType findFkeyCast(Oid targetTypeId, Oid sourceTypeId,
399399
static void validateForeignKeyConstraint(char *conname,
400400
Relation rel, Relation pkrel,
401401
Oid pkindOid, Oid constraintOid);
402+
static void CheckAlterTableIsSafe(Relation rel);
402403
static void ATController(AlterTableStmt *parsetree,
403404
Relation rel, List *cmds, bool recurse, LOCKMODE lockmode,
404405
AlterTableUtilityContext *context);
@@ -4269,6 +4270,37 @@ CheckTableNotInUse(Relation rel, const char *stmt)
42694270
stmt, RelationGetRelationName(rel))));
42704271
}
42714272

4273+
/*
4274+
* CheckAlterTableIsSafe
4275+
* Verify that it's safe to allow ALTER TABLE on this relation.
4276+
*
4277+
* This consists of CheckTableNotInUse() plus a check that the relation
4278+
* isn't another session's temp table. We must split out the temp-table
4279+
* check because there are callers of CheckTableNotInUse() that don't want
4280+
* that, notably DROP TABLE. (We must allow DROP or we couldn't clean out
4281+
* an orphaned temp schema.) Compare truncate_check_activity().
4282+
*/
4283+
static void
4284+
CheckAlterTableIsSafe(Relation rel)
4285+
{
4286+
/*
4287+
* Don't allow ALTER on temp tables of other backends. Their local buffer
4288+
* manager is not going to cope if we need to change the table's contents.
4289+
* Even if we don't, there may be optimizations that assume temp tables
4290+
* aren't subject to such interference.
4291+
*/
4292+
if (RELATION_IS_OTHER_TEMP(rel))
4293+
ereport(ERROR,
4294+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
4295+
errmsg("cannot alter temporary tables of other sessions")));
4296+
4297+
/*
4298+
* Also check for active uses of the relation in the current transaction,
4299+
* including open scans and pending AFTER trigger events.
4300+
*/
4301+
CheckTableNotInUse(rel, "ALTER TABLE");
4302+
}
4303+
42724304
/*
42734305
* AlterTableLookupRelation
42744306
* Look up, and lock, the OID for the relation named by an alter table
@@ -4342,7 +4374,7 @@ AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode,
43424374
/* Caller is required to provide an adequate lock. */
43434375
rel = relation_open(context->relid, NoLock);
43444376

4345-
CheckTableNotInUse(rel, "ALTER TABLE");
4377+
CheckAlterTableIsSafe(rel);
43464378

43474379
ATController(stmt, rel, stmt->cmds, stmt->relation->inh, lockmode, context);
43484380
}
@@ -5748,7 +5780,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
57485780

57495781
/*
57505782
* Don't allow rewrite on temp tables of other backends ... their
5751-
* local buffer manager is not going to cope.
5783+
* local buffer manager is not going to cope. (This is redundant
5784+
* with the check in CheckAlterTableIsSafe, but for safety we'll
5785+
* check here too.)
57525786
*/
57535787
if (RELATION_IS_OTHER_TEMP(OldHeap))
57545788
ereport(ERROR,
@@ -6619,7 +6653,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
66196653
continue;
66206654
/* find_all_inheritors already got lock */
66216655
childrel = relation_open(childrelid, NoLock);
6622-
CheckTableNotInUse(childrel, "ALTER TABLE");
6656+
CheckAlterTableIsSafe(childrel);
66236657
ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode, context);
66246658
relation_close(childrel, NoLock);
66256659
}
@@ -6628,7 +6662,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
66286662

66296663
/*
66306664
* Obtain list of partitions of the given table, locking them all at the given
6631-
* lockmode and ensuring that they all pass CheckTableNotInUse.
6665+
* lockmode and ensuring that they all pass CheckAlterTableIsSafe.
66326666
*
66336667
* This function is a no-op if the given relation is not a partitioned table;
66346668
* in particular, nothing is done if it's a legacy inheritance parent.
@@ -6649,7 +6683,7 @@ ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
66496683

66506684
/* find_all_inheritors already got lock */
66516685
childrel = table_open(lfirst_oid(cell), NoLock);
6652-
CheckTableNotInUse(childrel, "ALTER TABLE");
6686+
CheckAlterTableIsSafe(childrel);
66536687
table_close(childrel, NoLock);
66546688
}
66556689
list_free(inh);
@@ -6682,7 +6716,7 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
66826716
Relation childrel;
66836717

66846718
childrel = relation_open(childrelid, lockmode);
6685-
CheckTableNotInUse(childrel, "ALTER TABLE");
6719+
CheckAlterTableIsSafe(childrel);
66866720
ATPrepCmd(wqueue, childrel, cmd, true, true, lockmode, context);
66876721
relation_close(childrel, NoLock);
66886722
}
@@ -7354,7 +7388,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73547388

73557389
/* find_inheritance_children already got lock */
73567390
childrel = table_open(childrelid, NoLock);
7357-
CheckTableNotInUse(childrel, "ALTER TABLE");
7391+
CheckAlterTableIsSafe(childrel);
73587392

73597393
/* Find or create work queue entry for this table */
73607394
childtab = ATGetQueueEntry(wqueue, childrel);
@@ -9031,7 +9065,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
90319065

90329066
/* find_inheritance_children already got lock */
90339067
childrel = table_open(childrelid, NoLock);
9034-
CheckTableNotInUse(childrel, "ALTER TABLE");
9068+
CheckAlterTableIsSafe(childrel);
90359069

90369070
tuple = SearchSysCacheCopyAttName(childrelid, colName);
90379071
if (!HeapTupleIsValid(tuple)) /* shouldn't happen */
@@ -9514,7 +9548,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
95149548

95159549
/* find_inheritance_children already got lock */
95169550
childrel = table_open(childrelid, NoLock);
9517-
CheckTableNotInUse(childrel, "ALTER TABLE");
9551+
CheckAlterTableIsSafe(childrel);
95189552

95199553
/* Find or create work queue entry for this table */
95209554
childtab = ATGetQueueEntry(wqueue, childrel);
@@ -10343,7 +10377,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
1034310377
referenced;
1034410378
ListCell *cell;
1034510379

10346-
CheckTableNotInUse(partition, "ALTER TABLE");
10380+
CheckAlterTableIsSafe(partition);
1034710381

1034810382
attmap = build_attrmap_by_name(RelationGetDescr(partition),
1034910383
RelationGetDescr(rel),
@@ -12460,7 +12494,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
1246012494

1246112495
/* Must match lock taken by RemoveTriggerById: */
1246212496
frel = table_open(con->confrelid, AccessExclusiveLock);
12463-
CheckTableNotInUse(frel, "ALTER TABLE");
12497+
CheckAlterTableIsSafe(frel);
1246412498
table_close(frel, NoLock);
1246512499
}
1246612500

@@ -12537,7 +12571,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
1253712571

1253812572
/* find_inheritance_children already got lock */
1253912573
childrel = table_open(childrelid, NoLock);
12540-
CheckTableNotInUse(childrel, "ALTER TABLE");
12574+
CheckAlterTableIsSafe(childrel);
1254112575

1254212576
ScanKeyInit(&skey[0],
1254312577
Anum_pg_constraint_conrelid,
@@ -12840,7 +12874,7 @@ ATPrepAlterColumnType(List **wqueue,
1284012874

1284112875
/* find_all_inheritors already got lock */
1284212876
childrel = relation_open(childrelid, NoLock);
12843-
CheckTableNotInUse(childrel, "ALTER TABLE");
12877+
CheckAlterTableIsSafe(childrel);
1284412878

1284512879
/*
1284612880
* Verify that the child doesn't have any inherited definitions of

0 commit comments

Comments
 (0)