8000 Improve behavior of concurrent ALTER <relation> .. SET SCHEMA. · postgrespro/postgres_cluster@1da5c11 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1da5c11

Browse files
robertmhaasrobertmhaas
committed
Improve behavior of concurrent ALTER <relation> .. SET SCHEMA.
If the referrent of a name changes while we're waiting for the lock, we must recheck permissons. We also now check the relkind before locking, since it's easy to do that long the way. Patch by me; review by Noah Misch.
1 parent 74a1d4f commit 1da5c11

File tree

2 files changed

+70
-44
lines changed

2 files changed

+70
-44
lines changed

‎src/backend/commands/alter.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
191191
case OBJECT_TABLE:
192192
case OBJECT_VIEW:
193193
case OBJECT_FOREIGN_TABLE:
194-
CheckRelationOwnership(stmt->relation, true);
195194
AlterTableNamespace(stmt->relation, stmt->newschema,
196195
stmt->objectType, AccessExclusiveLock);
197196
break;

‎src/backend/commands/tablecmds.c‎

Lines changed: 70 additions & 43 deletions
< 2D03 td data-grid-cell-id="diff-d373fb790605aded212c2e3a0fbe8c015d110077ff46c07d0a6d6647a2d7e928-9479-9496-1" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionNum-bgColor, var(--diffBlob-addition-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">9496
Original file line numberDiff line numberDiff line change
@@ -9379,28 +9379,35 @@ ATExecGenericOptions(Relation rel, List *options)
93799379
heap_freetuple(tuple);
93809380
}
93819381

9382-
93839382
/*
9384-
* Execute ALTER TABLE SET SCHEMA
9385-
*
9386-
* Note: caller must have checked ownership of the relation already
9383+
* Perform permissions and integrity checks before acquiring a relation lock.
93879384
*/
9388-
void
9389-
AlterTableNamespace(RangeVar *relation, const char *newschema,
9390-
ObjectType stmttype, LOCKMODE lockmode)
9385+
static void
9386+
RangeVarCallbackForAlterTableNamespace(const RangeVar *rv, Oid relid,
9387+
Oid oldrelid, void *arg)
93919388
{
9392-
Relation rel;
9393-
Oid relid;
9394-
Oid oldNspOid;
9395-
Oid nspOid;
9396-
Relation classRel;
9389+
HeapTuple tuple;
9390+
Form_pg_class form;
9391+
ObjectType stmttype;
9392+
9393+
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
9394+
if (!HeapTupleIsValid(tuple))
9395+
return; /* concurrently dropped */
9396+
form = (Form_pg_class) GETSTRUCT(tuple);
93979397

9398-
rel = relation_openrv(relation, lockmode);
9398+
/* Must own table. */
9399+
if (!pg_class_ownercheck(relid, GetUserId()))
9400+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, rv->relname);
93999401

9400-
relid = RelationGetRelid(rel);
9401-
oldNspOid = RelationGetNamespace(rel);
9402+
/* No system table modifications unless explicitly allowed. */
9403+
if (!allowSystemTableMods && IsSystemClass(form))
9404+
ereport(ERROR,
9405+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
9406+
errmsg("permission denied: \"%s\" is a system catalog",
9407+
rv->relname)));
94029408

94039409
/* Check relation type against type specified in the ALTER command */
9410+
stmttype = * (ObjectType *) arg;
94049411
switch (stmttype)
94059412
{
94069413
case OBJECT_TABLE:
@@ -9412,61 +9419,43 @@ AlterTableNamespace(RangeVar *relation, const char *newschema,
94129419
break;
94139420

94149421
case OBJECT_SEQUENCE:
9415-
if (rel->rd_rel->relkind != RELKIND_SEQUENCE)
9422+
if (form->relkind != RELKIND_SEQUENCE)
94169423
ereport(ERROR,
94179424
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9418-
errmsg("\"%s\" is not a sequence",
9419-
RelationGetRelationName(rel))));
9425+
errmsg("\"%s\" is not a sequence", rv->relname)));
94209426
break;
94219427

94229428
case OBJECT_VIEW:
9423-
if (rel->rd_rel->relkind != RELKIND_VIEW)
9429+
if (form->relkind != RELKIND_VIEW)
94249430
ereport(ERROR,
94259431
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9426-
errmsg("\"%s\" is not a view",
9427-
RelationGetRelationName(rel))));
9432+
errmsg("\"%s\" is not a view", rv->relname)));
94289433
break;
94299434

94309435
case OBJECT_FOREIGN_TABLE:
9431-
if (rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE)
9436+
if (form->relkind != RELKIND_FOREIGN_TABLE)
94329437
ereport(ERROR,
94339438
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9434-
errmsg("\"%s\" is not a foreign table",
9435-
RelationGetRelationName(rel))));
9439+
errmsg("\"%s\" is not a foreign table", rv->relname)));
94369440
break;
94379441

94389442
default:
94399443
elog(ERROR, "unrecognized object type: %d", (int) stmttype);
94409444
}
94419445

94429446
/* Can we change the schema of this tuple? */
9443-
switch (rel->rd_rel->relkind)
9447+
switch (form->relkind)
94449448
{
94459449
case RELKIND_RELATION:
94469450
case RELKIND_VIEW:
9451+
case RELKIND_SEQUENCE:
94479452
case RELKIND_FOREIGN_TABLE:
94489453
/* ok to change schema */
94499454
break;
9450-
case RELKIND_SEQUENCE:
9451-
{
9452-
/* if it's an owned sequence, disallow moving it by itself */
9453-
Oid tableId;
9454-
int32 colId;
9455-
9456-
if (sequenceIsOwned(relid, &tableId, &colId))
9457-
ereport(ERROR,
9458-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
9459-
errmsg("cannot move an owned sequence into another schema"),
9460-
errdetail("Sequence \"%s\" is linked to table \"%s\".",
9461-
RelationGetRelationName(rel),
9462-
get_rel_name(tableId))));
9463-
}
9464-
break;
94659455
case RELKIND_COMPOSITE_TYPE:
94669456
ereport(ERROR,
94679457
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9468-
errmsg("\"%s\" is a composite type",
9469-
RelationGetRelationName(rel)),
9458+
errmsg("\"%s\" is a composite type", rv->relname),
94709459
errhint("Use ALTER TYPE instead.")));
94719460
break;
94729461
case RELKIND_INDEX:
@@ -9476,7 +9465,45 @@ AlterTableNamespace(RangeVar *relation, const char *newschema,
94769465
ereport(ERROR,
94779466
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
94789467
errmsg("\"%s\" is not a table, view, sequence, or foreign table",
9479-
RelationGetRelationName(rel))));
9468+
rv->relname)));
9469+
}
9470+
9471+
ReleaseSysCache(tuple);
9472+
}
9473+
9474+
/*
9475+
* Execute ALTER TABLE SET SCHEMA
9476+
*/
9477+
void
9478+
AlterTableNamespace(RangeVar *relation, const char *newschema,
9479+
ObjectType stmttype, LOCKMODE lockmode)
9480+
{
9481+
Relation rel;
9482+
Oid relid;
9483+
Oid oldNspOid;
9484+
Oid nspOid;
9485+
Relation classRel;
9486+
9487+
relid = RangeVarGetRelidExtended(relation, lockmode, false, false,
9488+
RangeVarCallbackForAlterTableNamespace,
9489+
(void *) &stmttype);
9490+
rel = relation_open(relid, NoLock);
9491+
9492+
oldNspOid = RelationGetNamespace(rel);
9493+
9494+
/* If it's an owned sequence, disallow moving it by itself. */
9495+
if (rel->rd_rel->relkind == RELKIND_SEQUENCE)
+
{
9497+
Oid tableId;
9498+
int32 colId;
9499+
9500+
if (sequenceIsOwned(relid, &tableId, &colId))
9501+
ereport(ERROR,
9502+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
9503+
errmsg("cannot move an owned sequence into another schema"),
9504+
errdetail("Sequence \"%s\" is linked to table \"%s\".",
9505+
RelationGetRelationName(rel),
9506+
get_rel_name(tableId))));
94809507
}
94819508

94829509
/* get schema OID and check its permissions */

0 commit comments

Comments
 (0)
0