mirror of
https://github.com/postgres/postgres.git
synced 2025-06-07 11:02:12 +03:00
Fix unsafe order of operations in foreign-table DDL commands.
When updating or deleting a system catalog tuple, it's necessary to acquire RowExclusiveLock on the catalog before looking up the tuple; otherwise a concurrent VACUUM FULL on the catalog might move the tuple to a different TID before we can apply the update. Coding patterns that find the tuple via a table scan aren't at risk here, but when obtaining the tuple from a catalog cache, correct ordering is important; and several routines in foreigncmds.c got it wrong. Noted while running the regression tests in parallel with VACUUM FULL of assorted system catalogs. For consistency I moved all the heap_open calls to the starts of their functions, including a couple for which there was no actual bug. Back-patch to 8.4 where foreigncmds.c was added.
This commit is contained in:
parent
00fc5d2263
commit
148f321eb4
@ -215,6 +215,8 @@ AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId)
|
|||||||
Oid fdwId;
|
Oid fdwId;
|
||||||
Form_pg_foreign_data_wrapper form;
|
Form_pg_foreign_data_wrapper form;
|
||||||
|
|
||||||
|
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
/* Must be a superuser to change a FDW owner */
|
/* Must be a superuser to change a FDW owner */
|
||||||
if (!superuser())
|
if (!superuser())
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
@ -231,8 +233,6 @@ AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId)
|
|||||||
name),
|
name),
|
||||||
errhint("The owner of a foreign-data wrapper must be a superuser.")));
|
errhint("The owner of a foreign-data wrapper must be a superuser.")));
|
||||||
|
|
||||||
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
tup = SearchSysCacheCopy1(FOREIGNDATAWRAPPERNAME, CStringGetDatum(name));
|
tup = SearchSysCacheCopy1(FOREIGNDATAWRAPPERNAME, CStringGetDatum(name));
|
||||||
|
|
||||||
if (!HeapTupleIsValid(tup))
|
if (!HeapTupleIsValid(tup))
|
||||||
@ -432,6 +432,8 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
|
|||||||
ObjectAddress myself;
|
ObjectAddress myself;
|
||||||
ObjectAddress referenced;
|
ObjectAddress referenced;
|
||||||
|
|
||||||
|
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
/* Must be super user */
|
/* Must be super user */
|
||||||
if (!superuser())
|
if (!superuser())
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
@ -455,8 +457,6 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
|
|||||||
/*
|
/*
|
||||||
* Insert tuple into pg_foreign_data_wrapper.
|
* Insert tuple into pg_foreign_data_wrapper.
|
||||||
*/
|
*/
|
||||||
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
memset(values, 0, sizeof(values));
|
memset(values, 0, sizeof(values));
|
||||||
memset(nulls, false, sizeof(nulls));
|
memset(nulls, false, sizeof(nulls));
|
||||||
|
|
||||||
@ -545,6 +545,8 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
|
|||||||
Oid fdwhandler;
|
Oid fdwhandler;
|
||||||
Oid fdwvalidator;
|
Oid fdwvalidator;
|
||||||
|
|
||||||
|
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
/* Must be super user */
|
/* Must be super user */
|
||||||
if (!superuser())
|
if (!superuser())
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
@ -635,9 +637,6 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Everything looks good - update the tuple */
|
/* Everything looks good - update the tuple */
|
||||||
|
|
||||||
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
||||||
repl_val, repl_null, repl_repl);
|
repl_val, repl_null, repl_repl);
|
||||||
|
|
||||||
@ -773,6 +772,8 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
|
|||||||
ObjectAddress referenced;
|
ObjectAddress referenced;
|
||||||
ForeignDataWrapper *fdw;
|
ForeignDataWrapper *fdw;
|
||||||
|
|
||||||
|
rel = heap_open(ForeignServerRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
/* For now the owner cannot be specified on create. Use effective user ID. */
|
/* For now the owner cannot be specified on create. Use effective user ID. */
|
||||||
ownerId = GetUserId();
|
ownerId = GetUserId();
|
||||||
|
|
||||||
@ -798,8 +799,6 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
|
|||||||
/*
|
/*
|
||||||
* Insert tuple into pg_foreign_server.
|
* Insert tuple into pg_foreign_server.
|
||||||
*/
|
*/
|
||||||
rel = heap_open(ForeignServerRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
memset(values, 0, sizeof(values));
|
memset(values, 0, sizeof(values));
|
||||||
memset(nulls, false, sizeof(nulls));
|
memset(nulls, false, sizeof(nulls));
|
||||||
|
|
||||||
@ -880,6 +879,8 @@ AlterForeignServer(AlterForeignServerStmt *stmt)
|
|||||||
Oid srvId;
|
Oid srvId;
|
||||||
Form_pg_foreign_server srvForm;
|
Form_pg_foreign_server srvForm;
|
||||||
|
|
||||||
|
rel = heap_open(ForeignServerRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
tp = SearchSysCacheCopy1(FOREIGNSERVERNAME,
|
tp = SearchSysCacheCopy1(FOREIGNSERVERNAME,
|
||||||
CStringGetDatum(stmt->servername));
|
CStringGetDatum(stmt->servername));
|
||||||
|
|
||||||
@ -945,9 +946,6 @@ AlterForeignServer(AlterForeignServerStmt *stmt)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Everything looks good - update the tuple */
|
/* Everything looks good - update the tuple */
|
||||||
|
|
||||||
rel = heap_open(ForeignServerRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
||||||
repl_val, repl_null, repl_repl);
|
repl_val, repl_null, repl_repl);
|
||||||
|
|
||||||
@ -1068,6 +1066,8 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
|
|||||||
ForeignServer *srv;
|
ForeignServer *srv;
|
||||||
ForeignDataWrapper *fdw;
|
ForeignDataWrapper *fdw;
|
||||||
|
|
||||||
|
rel = heap_open(UserMappingRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
useId = GetUserOidFromMapping(stmt->username, false);
|
useId = GetUserOidFromMapping(stmt->username, false);
|
||||||
|
|
||||||
/* Check that the server exists. */
|
/* Check that the server exists. */
|
||||||
@ -1093,8 +1093,6 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
|
|||||||
/*
|
/*
|
||||||
* Insert tuple into pg_user_mapping.
|
* Insert tuple into pg_user_mapping.
|
||||||
*/
|
*/
|
||||||
rel = heap_open(UserMappingRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
memset(values, 0, sizeof(values));
|
memset(values, 0, sizeof(values));
|
||||||
memset(nulls, false, sizeof(nulls));
|
memset(nulls, false, sizeof(nulls));
|
||||||
|
|
||||||
@ -1161,6 +1159,8 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
|
|||||||
Oid umId;
|
Oid umId;
|
||||||
ForeignServer *srv;
|
ForeignServer *srv;
|
||||||
|
|
||||||
|
rel = heap_open(UserMappingRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
useId = GetUserOidFromMapping(stmt->username, false);
|
useId = GetUserOidFromMapping(stmt->username, false);
|
||||||
srv = GetForeignServerByName(stmt->servername, false);
|
srv = GetForeignServerByName(stmt->servername, false);
|
||||||
|
|
||||||
@ -1218,9 +1218,6 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Everything looks good - update the tuple */
|
/* Everything looks good - update the tuple */
|
||||||
|
|
||||||
rel = heap_open(UserMappingRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
tp = heap_modify_tuple(tp, RelationGetDescr(rel),
|
||||||
repl_val, repl_null, repl_repl);
|
repl_val, repl_null, repl_repl);
|
||||||
|
|
||||||
@ -1344,11 +1341,13 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
|
|||||||
ForeignServer *server;
|
ForeignServer *server;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Advance command counter to ensure the pg_attribute tuple visible; the
|
* Advance command counter to ensure the pg_attribute tuple is visible;
|
||||||
* tuple might be updated to add constraints in previous step.
|
* the tuple might be updated to add constraints in previous step.
|
||||||
*/
|
*/
|
||||||
CommandCounterIncrement();
|
CommandCounterIncrement();
|
||||||
|
|
||||||
|
ftrel = heap_open(ForeignTableRelationId, RowExclusiveLock);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* For now the owner cannot be specified on create. Use effective user ID.
|
* For now the owner cannot be specified on create. Use effective user ID.
|
||||||
*/
|
*/
|
||||||
@ -1368,8 +1367,6 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
|
|||||||
/*
|
/*
|
||||||
* Insert tuple into pg_foreign_table.
|
* Insert tuple into pg_foreign_table.
|
||||||
*/
|
*/
|
||||||
ftrel = heap_open(ForeignTableRelationId, RowExclusiveLock);
|
|
||||||
|
|
||||||
memset(values, 0, sizeof(values));
|
memset(values, 0, sizeof(values));
|
||||||
memset(nulls, false, sizeof(nulls));
|
memset(nulls, false, sizeof(nulls));
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user