Commit 52994e9e authored by Tom Lane's avatar Tom Lane

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.
parent 85612039
...@@ -215,6 +215,8 @@ AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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) ...@@ -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));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment