Skip to content

Commit 1299564

Browse files
nmischnmisch
committed
Back-patch "Refactor code in tablecmds.c to check and process tablespace moves"
Back-patch commits 4c9c359 and 2484329 to v13 and v12. Before those commits, we held the modifiable copy of the relation's pg_class row throughout a table_relation_copy_data(). That can last long enough to copy MaxBlockNumber of data. A subsequent fix will hold LockTuple() for the lifespan of that modifiable copy. By back-patching this first, we avoid a needless long-duration LOCKTAG_TUPLE. Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com
1 parent e43e71b commit 1299564

File tree

2 files changed

+123
-91
lines changed

2 files changed

+123
-91
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 119 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -3069,6 +3069,112 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)
30693069
table_close(relationRelation, RowExclusiveLock);
30703070
}
30713071

3072+
/*
3073+
* CheckRelationTableSpaceMove
3074+
* Check if relation can be moved to new tablespace.
3075+
*
3076+
* NOTE: The caller must hold AccessExclusiveLock on the relation.
3077+
*
3078+
* Returns true if the relation can be moved to the new tablespace; raises
3079+
* an error if it is not possible to do the move; returns false if the move
3080+
* would have no effect.
3081+
*/
3082+
bool
3083+
CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)
3084+
{
3085+
Oid oldTableSpaceId;
3086+
3087+
/*
3088+
* No work if no change in tablespace. Note that MyDatabaseTableSpace is
3089+
* stored as 0.
3090+
*/
3091+
oldTableSpaceId = rel->rd_rel->reltablespace;
3092+
if (newTableSpaceId == oldTableSpaceId ||
3093+
(newTableSpaceId == MyDatabaseTableSpace && oldTableSpaceId == 0))
3094+
return false;
3095+
3096+
/*
3097+
* We cannot support moving mapped relations into different tablespaces.
3098+
* (In particular this eliminates all shared catalogs.)
3099+
*/
3100+
if (RelationIsMapped(rel))
3101+
ereport(ERROR,
3102+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3103+
errmsg("cannot move system relation \"%s\"",
3104+
RelationGetRelationName(rel))));
3105+
3106+
/* Cannot move a non-shared relation into pg_global */
3107+
if (newTableSpaceId == GLOBALTABLESPACE_OID)
3108+
ereport(ERROR,
3109+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
3110+
errmsg("only shared relations can be placed in pg_global tablespace")));
3111+
3112+
/*
3113+
* Do not allow moving temp tables of other backends ... their local
3114+
* buffer manager is not going to cope.
3115+
*/
3116+
if (RELATION_IS_OTHER_TEMP(rel))
3117+
ereport(ERROR,
3118+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3119+
errmsg("cannot move temporary tables of other sessions")));
3120+
3121+
return true;
3122+
}
3123+
3124+
/*
3125+
* SetRelationTableSpace
3126+
* Set new reltablespace and relfilenode in pg_class entry.
3127+
*
3128+
* newTableSpaceId is the new tablespace for the relation, and
3129+
* newRelFileNode its new filenode. If newRelFileNode is InvalidOid,
3130+
* this field is not updated.
3131+
*
3132+
* NOTE: The caller must hold AccessExclusiveLock on the relation.
3133+
*
3134+
* The caller of this routine had better check if a relation can be
3135+
* moved to this new tablespace by calling CheckRelationTableSpaceMove()
3136+
* first, and is responsible for making the change visible with
3137+
* CommandCounterIncrement().
3138+
*/
3139+
void
3140+
SetRelationTableSpace(Relation rel,
3141+
Oid newTableSpaceId,
3142+
Oid newRelFileNode)
3143+
{
3144+
Relation pg_class;
3145+
HeapTuple tuple;
3146+
Form_pg_class rd_rel;
3147+
Oid reloid = RelationGetRelid(rel);
3148+
3149+
Assert(CheckRelationTableSpaceMove(rel, newTableSpaceId));
3150+
3151+
/* Get a modifiable copy of the relation's pg_class row. */
3152+
pg_class = table_open(RelationRelationId, RowExclusiveLock);
3153+
3154+
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
3155+
if (!HeapTupleIsValid(tuple))
3156+
elog(ERROR, "cache lookup failed for relation %u", reloid);
3157+
rd_rel = (Form_pg_class) GETSTRUCT(tuple);
3158+
3159+
/* Update the pg_class row. */
3160+
rd_rel->reltablespace = (newTableSpaceId == MyDatabaseTableSpace) ?
3161+
InvalidOid : newTableSpaceId;
3162+
if (OidIsValid(newRelFileNode))
3163+
rd_rel->relfilenode = newRelFileNode;
3164+
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
3165+
3166+
/*
3167+
* Record dependency on tablespace. This is only required for relations
3168+
* that have no physical storage.
3169+
*/
3170+
if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
3171+
changeDependencyOnTablespace(RelationRelationId, reloid,
3172+
rd_rel->reltablespace);
3173+
3174+
heap_freetuple(tuple);
3175+
table_close(pg_class, RowExclusiveLock);
3176+
}
3177+
30723178
/*
30733179
* renameatt_check - basic sanity checks before attribute rename
30743180
*/
@@ -13565,13 +13671,9 @@ static void
1356513671
ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
1356613672
{
1356713673
Relation rel;
13568-
Oid oldTableSpace;
1356913674
Oid reltoastrelid;
1357013675
Oid newrelfilenode;
1357113676
RelFileNode newrnode;
13572-
Relation pg_class;
13573-
HeapTuple tuple;
13574-
Form_pg_class rd_rel;
1357513677
List *reltoastidxids = NIL;
1357613678
ListCell *lc;
1357713679

@@ -13580,45 +13682,15 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
1358013682
*/
1358113683
rel = relation_open(tableOid, lockmode);
1358213684

13583-
/*
13584-
* No work if no change in tablespace.
13585-
*/
13586-
oldTableSpace = rel->rd_rel->reltablespace;
13587-
if (newTableSpace == oldTableSpace ||
13588-
(newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
13685+
/* Check first if relation can be moved to new tablespace */
13686+
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
1358913687
{
1359013688
InvokeObjectPostAlterHook(RelationRelationId,
1359113689
RelationGetRelid(rel), 0);
13592-
1359313690
relation_close(rel, NoLock);
1359413691
return;
1359513692
}
1359613693

13597-
/*
13598-
* We cannot support moving mapped relations into different tablespaces.
13599-
* (In particular this eliminates all shared catalogs.)
13600-
*/
13601-
if (RelationIsMapped(rel))
13602-
ereport(ERROR,
13603-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
13604-
errmsg("cannot move system relation \"%s\"",
13605-
RelationGetRelationName(rel))));
13606-
13607-
/* Can't move a non-shared relation into pg_global */
13608-
if (newTableSpace == GLOBALTABLESPACE_OID)
13609-
ereport(ERROR,
13610-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
13611-
errmsg("only shared relations can be placed in pg_global tablespace")));
13612-
13613-
/*
13614-
* Don't allow moving temp tables of other backends ... their local buffer
13615-
* manager is not going to cope.
13616-
*/
13617-
if (RELATION_IS_OTHER_TEMP(rel))
13618-
ereport(ERROR,
13619-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
13620-
errmsg("cannot move temporary tables of other sessions")));
13621-
1362213694
reltoastrelid = rel->rd_rel->reltoastrelid;
1362313695
/* Fetch the list of indexes on toast relation if necessary */
1362413696
if (OidIsValid(reltoastrelid))
@@ -13629,14 +13701,6 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
1362913701
relation_close(toastRel, lockmode);
1363013702
}
1363113703

13632-
/* Get a modifiable copy of the relation's pg_class row */
13633-
pg_class = table_open(RelationRelationId, RowExclusiveLock);
13634-
13635-
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(tableOid));
13636-
if (!HeapTupleIsValid(tuple))
13637-
elog(ERROR, "cache lookup failed for relation %u", tableOid);
13638-
rd_rel = (Form_pg_class) GETSTRUCT(tuple);
13639-
1364013704
/*
1364113705
* Relfilenodes are not unique in databases across tablespaces, so we need
1364213706
* to allocate a new one in the new tablespace.
@@ -13667,18 +13731,13 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
1366713731
*
1366813732
* NB: This wouldn't work if ATExecSetTableSpace() were allowed to be
1366913733
* executed on pg_class or its indexes (the above copy wouldn't contain
13670-
* the updated pg_class entry), but that's forbidden above.
13734+
* the updated pg_class entry), but that's forbidden with
13735+
* CheckRelationTableSpaceMove().
1367113736
*/
13672-
rd_rel->reltablespace = (newTableSpace == MyDatabaseTableSpace) ? InvalidOid : newTableSpace;
13673-
rd_rel->relfilenode = newrelfilenode;
13674-
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
13737+
SetRelationTableSpace(rel, newTableSpace, newrelfilenode);
1367513738

1367613739
InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);
1367713740

13678-
heap_freetuple(tuple);
13679-
13680-
table_close(pg_class, RowExclusiveLock);
13681-
1368213741
RelationAssumeNewRelfilenode(rel);
1368313742

1368413743
relation_close(rel, NoLock);
@@ -13706,56 +13765,25 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
1370613765
static void
1370713766
ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
1370813767
{
13709-
HeapTuple tuple;
13710-
Oid oldTableSpace;
13711-
Relation pg_class;
13712-
Form_pg_class rd_rel;
13713-
Oid reloid = RelationGetRelid(rel);
13714-
1371513768
/*
1371613769
* Shouldn't be called on relations having storage; these are processed in
1371713770
* phase 3.
1371813771
*/
1371913772
Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind));
1372013773

13721-
/* Can't allow a non-shared relation in pg_global */
13722-
if (newTableSpace == GLOBALTABLESPACE_OID)
13723-
ereport(ERROR,
13724-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
13725-
errmsg("only shared relations can be placed in pg_global tablespace")));
13726-
13727-
/*
13728-
* No work if no change in tablespace.
13729-
*/
13730-
oldTableSpace = rel->rd_rel->reltablespace;
13731-
if (newTableSpace == oldTableSpace ||
13732-
(newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
13774+
/* check if relation can be moved to its new tablespace */
13775+
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
1373313776
{
13734-
InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
13777+
InvokeObjectPostAlterHook(RelationRelationId,
13778+
RelationGetRelid(rel),
13779+
0);
1373513780
return;
1373613781
}
1373713782

13738-
/* Get a modifiable copy of the relation's pg_class row */
13739-
pg_class = table_open(RelationRelationId, RowExclusiveLock);
13740-
13741-
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
13742-
if (!HeapTupleIsValid(tuple))
13743-
elog(ERROR, "cache lookup failed for relation %u", reloid);
13744-
rd_rel = (Form_pg_class) GETSTRUCT(tuple);
13745-
13746-
/* update the pg_class row */
13747-
rd_rel->reltablespace = (newTableSpace == MyDatabaseTableSpace) ? InvalidOid : newTableSpace;
13748-
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
13749-
13750-
/* Record dependency on tablespace */
13751-
changeDependencyOnTablespace(RelationRelationId,
13752-
reloid, rd_rel->reltablespace);
13753-
13754-
InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
13783+
/* Update can be done, so change reltablespace */
13784+
SetRelationTableSpace(rel, newTableSpace, InvalidOid);
1375513785

13756-
heap_freetuple(tuple);
13757-
13758-
table_close(pg_class, RowExclusiveLock);
13786+
InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);
1375913787

1376013788
/* Make sure the reltablespace change is visible */
1376113789
CommandCounterIncrement();

‎src/include/commands/tablecmds.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ extern void ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_
6161

6262
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
6363

64+
extern bool CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId);
65+
extern void SetRelationTableSpace(Relation rel, Oid newTableSpaceId,
66+
Oid newRelFileNode);
67+
6468
extern ObjectAddress renameatt(RenameStmt *stmt);
6569

6670
extern ObjectAddress RenameConstraint(RenameStmt *stmt);

0 commit comments

Comments
 (0)