mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-04 04:37:20 +02:00
Ensure correct lock level is used in ALTER ... RENAME
Commit 1b5d797cd4
intended to relax the lock level used to rename
indexes, but inadvertently allowed *any* relation to be renamed with a
lowered lock level, as long as the command is spelled ALTER INDEX.
That's undesirable for other relation types, so retry the operation with
the higher lock if the relation turns out not to be an index.
After this fix, ALTER INDEX <sometable> RENAME will require access
exclusive lock, which it didn't before.
Author: Nathan Bossart <bossartn@amazon.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Onder Kalaci <onderk@microsoft.com>
Discussion: https://postgr.es/m/PH0PR21MB1328189E2821CDEC646F8178D8AE9@PH0PR21MB1328.namprd21.prod.outlook.com
This commit is contained in:
parent
9aef401557
commit
6c8d1c197b
@ -3406,7 +3406,7 @@ RenameConstraint(RenameStmt *stmt)
|
|||||||
ObjectAddress
|
ObjectAddress
|
||||||
RenameRelation(RenameStmt *stmt)
|
RenameRelation(RenameStmt *stmt)
|
||||||
{
|
{
|
||||||
bool is_index = stmt->renameType == OBJECT_INDEX;
|
bool is_index_stmt = stmt->renameType == OBJECT_INDEX;
|
||||||
Oid relid;
|
Oid relid;
|
||||||
ObjectAddress address;
|
ObjectAddress address;
|
||||||
|
|
||||||
@ -3416,10 +3416,18 @@ RenameRelation(RenameStmt *stmt)
|
|||||||
* end of transaction.
|
* end of transaction.
|
||||||
*
|
*
|
||||||
* Lock level used here should match RenameRelationInternal, to avoid lock
|
* Lock level used here should match RenameRelationInternal, to avoid lock
|
||||||
* escalation.
|
* escalation. However, because ALTER INDEX can be used with any relation
|
||||||
|
* type, we mustn't believe without verification.
|
||||||
*/
|
*/
|
||||||
relid = RangeVarGetRelidExtended(stmt->relation,
|
for (;;)
|
||||||
is_index ? ShareUpdateExclusiveLock : AccessExclusiveLock,
|
{
|
||||||
|
LOCKMODE lockmode;
|
||||||
|
char relkind;
|
||||||
|
bool obj_is_index;
|
||||||
|
|
||||||
|
lockmode = is_index_stmt ? ShareUpdateExclusiveLock : AccessExclusiveLock;
|
||||||
|
|
||||||
|
relid = RangeVarGetRelidExtended(stmt->relation, lockmode,
|
||||||
stmt->missing_ok ? RVR_MISSING_OK : 0,
|
stmt->missing_ok ? RVR_MISSING_OK : 0,
|
||||||
RangeVarCallbackForAlterRelation,
|
RangeVarCallbackForAlterRelation,
|
||||||
(void *) stmt);
|
(void *) stmt);
|
||||||
@ -3432,8 +3440,24 @@ RenameRelation(RenameStmt *stmt)
|
|||||||
return InvalidObjectAddress;
|
return InvalidObjectAddress;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* We allow mismatched statement and object types (e.g., ALTER INDEX
|
||||||
|
* to rename a table), but we might've used the wrong lock level. If
|
||||||
|
* that happens, retry with the correct lock level. We don't bother
|
||||||
|
* if we already acquired AccessExclusiveLock with an index, however.
|
||||||
|
*/
|
||||||
|
relkind = get_rel_relkind(relid);
|
||||||
|
obj_is_index = (relkind == RELKIND_INDEX ||
|
||||||
|
relkind == RELKIND_PARTITIONED_INDEX);
|
||||||
|
if (obj_is_index || is_index_stmt == obj_is_index)
|
||||||
|
break;
|
||||||
|
|
||||||
|
UnlockRelationOid(relid, lockmode);
|
||||||
|
is_index_stmt = obj_is_index;
|
||||||
|
}
|
||||||
|
|
||||||
/* Do the work */
|
/* Do the work */
|
||||||
RenameRelationInternal(relid, stmt->newname, false, is_index);
|
RenameRelationInternal(relid, stmt->newname, false, is_index_stmt);
|
||||||
|
|
||||||
ObjectAddressSet(address, RelationRelationId, relid);
|
ObjectAddressSet(address, RelationRelationId, relid);
|
||||||
|
|
||||||
@ -3481,6 +3505,16 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
|
|||||||
errmsg("relation \"%s\" already exists",
|
errmsg("relation \"%s\" already exists",
|
||||||
newrelname)));
|
newrelname)));
|
||||||
|
|
||||||
|
/*
|
||||||
|
* RenameRelation is careful not to believe the caller's idea of the
|
||||||
|
* relation kind being handled. We don't have to worry about this, but
|
||||||
|
* let's not be totally oblivious to it. We can process an index as
|
||||||
|
* not-an-index, but not the other way around.
|
||||||
|
*/
|
||||||
|
Assert(!is_index ||
|
||||||
|
is_index == (targetrelation->rd_rel->relkind == RELKIND_INDEX ||
|
||||||
|
targetrelation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Update pg_class tuple with new relname. (Scribbling on reltup is OK
|
* Update pg_class tuple with new relname. (Scribbling on reltup is OK
|
||||||
* because it's a copy...)
|
* because it's a copy...)
|
||||||
|
@ -232,6 +232,54 @@ SET ROLE regress_alter_table_user1;
|
|||||||
ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied
|
ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied
|
||||||
ERROR: must be owner of index onek_unique1
|
ERROR: must be owner of index onek_unique1
|
||||||
RESET ROLE;
|
RESET ROLE;
|
||||||
|
-- rename statements with mismatching statement and object types
|
||||||
|
CREATE TABLE alter_idx_rename_test (a INT);
|
||||||
|
CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a);
|
||||||
|
CREATE TABLE alter_idx_rename_test_parted (a INT) PARTITION BY LIST (a);
|
||||||
|
CREATE INDEX alter_idx_rename_test_parted_idx ON alter_idx_rename_test_parted (a);
|
||||||
|
BEGIN;
|
||||||
|
ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
|
||||||
|
ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
relation | mode
|
||||||
|
--------------------------------+---------------------
|
||||||
|
alter_idx_rename_test_2 | AccessExclusiveLock
|
||||||
|
alter_idx_rename_test_parted_2 | AccessExclusiveLock
|
||||||
|
(2 rows)
|
||||||
|
|
||||||
|
COMMIT;
|
||||||
|
BEGIN;
|
||||||
|
ALTER INDEX alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
|
||||||
|
ALTER INDEX alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
relation | mode
|
||||||
|
------------------------------------+--------------------------
|
||||||
|
alter_idx_rename_test_idx_2 | ShareUpdateExclusiveLock
|
||||||
|
alter_idx_rename_test_parted_idx_2 | ShareUpdateExclusiveLock
|
||||||
|
(2 rows)
|
||||||
|
|
||||||
|
COMMIT;
|
||||||
|
BEGIN;
|
||||||
|
ALTER TABLE alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
|
||||||
|
ALTER TABLE alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
relation | mode
|
||||||
|
------------------------------------+---------------------
|
||||||
|
alter_idx_rename_test_idx_3 | AccessExclusiveLock
|
||||||
|
alter_idx_rename_test_parted_idx_3 | AccessExclusiveLock
|
||||||
|
(2 rows)
|
||||||
|
|
||||||
|
COMMIT;
|
||||||
|
DROP TABLE alter_idx_rename_test_2;
|
||||||
-- renaming views
|
-- renaming views
|
||||||
CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1;
|
CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1;
|
||||||
ALTER TABLE attmp_view RENAME TO attmp_view_new;
|
ALTER TABLE attmp_view RENAME TO attmp_view_new;
|
||||||
|
@ -227,6 +227,37 @@ SET ROLE regress_alter_table_user1;
|
|||||||
ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied
|
ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied
|
||||||
RESET ROLE;
|
RESET ROLE;
|
||||||
|
|
||||||
|
-- rename statements with mismatching statement and object types
|
||||||
|
CREATE TABLE alter_idx_rename_test (a INT);
|
||||||
|
CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a);
|
||||||
|
CREATE TABLE alter_idx_rename_test_parted (a INT) PARTITION BY LIST (a);
|
||||||
|
CREATE INDEX alter_idx_rename_test_parted_idx ON alter_idx_rename_test_parted (a);
|
||||||
|
BEGIN;
|
||||||
|
ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
|
||||||
|
ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
COMMIT;
|
||||||
|
BEGIN;
|
||||||
|
ALTER INDEX alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
|
||||||
|
ALTER INDEX alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
COMMIT;
|
||||||
|
BEGIN;
|
||||||
|
ALTER TABLE alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
|
||||||
|
ALTER TABLE alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
|
||||||
|
SELECT relation::regclass, mode FROM pg_locks
|
||||||
|
WHERE pid = pg_backend_pid() AND locktype = 'relation'
|
||||||
|
AND relation::regclass::text LIKE 'alter\_idx%'
|
||||||
|
ORDER BY relation::regclass::text;
|
||||||
|
COMMIT;
|
||||||
|
DROP TABLE alter_idx_rename_test_2;
|
||||||
|
|
||||||
-- renaming views
|
-- renaming views
|
||||||
CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1;
|
CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1;
|
||||||
ALTER TABLE attmp_view RENAME TO attmp_view_new;
|
ALTER TABLE attmp_view RENAME TO attmp_view_new;
|
||||||
|
Loading…
Reference in New Issue
Block a user