NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit74a1d4f

committed

Improve behavior of concurrent rename statements.

Previously, renaming a table, sequence, view, index, foreign table,column, or trigger checked permissions before locking the object, whichmeant that if permissions were revoked during the lock wait, we wouldstill allow the operation. Similarly, if the original object is droppedand a new one with the same name is created, the operation will be allowedif we had permissions on the old object; the permissions on the newobject don't matter. All this is now fixed.Along the way, attempting to rename a trigger on a foreign table now givesthe same error message as trying to create one there in the first place(i.e. that it's not a table or view) rather than simply stating that notrigger by that name exists.Patch by me; review by Noah Misch.

1 parentd039fd5 commit74a1d4fCopy full SHA for 74a1d4f

File tree

7 files changed

+192

-141

lines changed

src
- backend/commands
- include/commands
  - tablecmds.h
  - trigger.h

7 files changed

+192

-141

lines changed

`‎src/backend/commands/alter.c`

Lines changed: 9 additions & 53 deletions

Original file line number	Diff line number	Diff line change
`@@ -105,62 +105,18 @@ ExecRenameStmt(RenameStmt *stmt)`
`105`	`105`	`caseOBJECT_SEQUENCE:`
`106`	`106`	`caseOBJECT_VIEW:`
`107`	`107`	`caseOBJECT_INDEX:`
	`108`	`+caseOBJECT_FOREIGN_TABLE:`
	`109`	`+RenameRelation(stmt);`
	`110`	`+break;`
	`111`	`+`
`108`	`112`	`caseOBJECT_COLUMN:`
`109`	`113`	`caseOBJECT_ATTRIBUTE:`
	`114`	`+renameatt(stmt);`
	`115`	`+break;`
	`116`	`+`
`110`	`117`	`caseOBJECT_TRIGGER:`
`111`		`-caseOBJECT_FOREIGN_TABLE:`
`112`		`-{`
`113`		`-Oidrelid;`
`114`		`-`
`115`		`-CheckRelationOwnership(stmt->relation, true);`
`116`		`-`
`117`		`-/*`
`118`		`- * Lock level used here should match what will be taken later,`
`119`		`- * in RenameRelation, renameatt, or renametrig.`
`120`		`- */`
`121`		`-relid=RangeVarGetRelid(stmt->relation,AccessExclusiveLock,`
`122`		`- false);`
`123`		`-`
`124`		`-switch (stmt->renameType)`
`125`		`-{`
`126`		`-caseOBJECT_TABLE:`
`127`		`-caseOBJECT_SEQUENCE:`
`128`		`-caseOBJECT_VIEW:`
`129`		`-caseOBJECT_INDEX:`
`130`		`-caseOBJECT_FOREIGN_TABLE:`
`131`		`-{`
`132`		`-/*`
`133`		`- * RENAME TABLE requires that we (still) hold`
`134`		`- * CREATE rights on the containing namespace, as`
`135`		`- * well as ownership of the table.`
`136`		`- */`
`137`		`-OidnamespaceId=get_rel_namespace(relid);`
`138`		`-AclResultaclresult;`
`139`		`-`
`140`		`-aclresult=pg_namespace_aclcheck(namespaceId,`
`141`		`-GetUserId(),`
`142`		`-ACL_CREATE);`
`143`		`-if (aclresult!=ACLCHECK_OK)`
`144`		`-aclcheck_error(aclresult,ACL_KIND_NAMESPACE,`
`145`		`-get_namespace_name(namespaceId));`
`146`		`-`
`147`		`-RenameRelation(relid,stmt->newname,stmt->renameType);`
`148`		`-break;`
`149`		`-}`
`150`		`-caseOBJECT_COLUMN:`
`151`		`-caseOBJECT_ATTRIBUTE:`
`152`		`-renameatt(relid,stmt);`
`153`		`-break;`
`154`		`-caseOBJECT_TRIGGER:`
`155`		`-renametrig(relid,`
`156`		`-stmt->subname,/* old att name */`
`157`		`-stmt->newname);/* new att name */`
`158`		`-break;`
`159`		`-default:`
`160`		`-/* can't happen */ ;`
`161`		`-}`
`162`		`-break;`
`163`		`-}`
	`118`	`+renametrig(stmt);`
	`119`	`+break;`
`164`	`120`
`165`	`121`	`caseOBJECT_TSPARSER:`
`166`	`122`	`RenameTSParser(stmt->object,stmt->newname);`

`‎src/backend/commands/cluster.c`

Lines changed: 2 additions & 6 deletions

Original file line number	Diff line number	Diff line change
`@@ -1474,28 +1474,24 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,`
`1474`	`1474`	`{`
`1475`	`1475`	`Relationtoastrel;`
`1476`	`1476`	`Oidtoastidx;`
`1477`		`-Oidtoastnamespace;`
`1478`	`1477`	`charNewToastName[NAMEDATALEN];`
`1479`	`1478`
`1480`	`1479`	`toastrel=relation_open(newrel->rd_rel->reltoastrelid,`
`1481`	`1480`	`AccessShareLock);`
`1482`	`1481`	`toastidx=toastrel->rd_rel->reltoastidxid;`
`1483`		`-toastnamespace=toastrel->rd_rel->relnamespace;`
`1484`	`1482`	`relation_close(toastrel,AccessShareLock);`
`1485`	`1483`
`1486`	`1484`	`/* rename the toast table ... */`
`1487`	`1485`	`snprintf(NewToastName,NAMEDATALEN,"pg_toast_%u",`
`1488`	`1486`	`OIDOldHeap);`
`1489`	`1487`	`RenameRelationInternal(newrel->rd_rel->reltoastrelid,`
`1490`		`-NewToastName,`
`1491`		`-toastnamespace);`
	`1488`	`+NewToastName);`
`1492`	`1489`
`1493`	`1490`	`/* ... and its index too */`
`1494`	`1491`	`snprintf(NewToastName,NAMEDATALEN,"pg_toast_%u_index",`
`1495`	`1492`	`OIDOldHeap);`
`1496`	`1493`	`RenameRelationInternal(toastidx,`
`1497`		`-NewToastName,`
`1498`		`-toastnamespace);`
	`1494`	`+NewToastName);`
`1499`	`1495`	`}`
`1500`	`1496`	`relation_close(newrel,NoLock);`
`1501`	`1497`	`}`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit74a1d4f

File tree

7 files changed

7 files changed

`‎src/backend/commands/alter.c`

`‎src/backend/commands/cluster.c`

0 commit comments