Skip to content

Commit 8725958

Browse files
alvherrealvherre
committed
Fix tablespace inheritance for partitioned rels
Commit ca41030 left a few loose ends. The most important one (broken pg_dump output) is already fixed by virtue of commit 3b23552, but some things remained: * When ALTER TABLE rewrites tables, the indexes must remain in the tablespace they were originally in. This didn't work because index recreation during ALTER TABLE runs manufactured SQL (yuck), which runs afoul of default_tablespace in competition with the parent relation tablespace. To fix, reset default_tablespace to the empty string temporarily, and add the TABLESPACE clause as appropriate. * Setting a partitioned rel's tablespace to the database default is confusing; if it worked, it would direct the partitions to that tablespace regardless of default_tablespace. But in reality it does not work, and making it work is a larger project. Therefore, throw an error when this condition is detected, to alert the unwary. Add some docs and tests, too. Author: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com
1 parent 3b23552 commit 8725958

File tree

18 files changed

+519
-34
lines changed

18 files changed

+519
-34
lines changed

‎doc/src/sgml/config.sgml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7356,7 +7356,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
73567356
<para>
73577357
This variable specifies the default tablespace in which to create
73587358
objects (tables and indexes) when a <command>CREATE</command> command does
7359-
not explicitly specify a tablespace.
7359+
not explicitly specify a tablespace. It also determines the tablespace
7360+
that a partitioned relation will direct future partitions to.
73607361
</para>
73617362

73627363
<para>

‎doc/src/sgml/ref/create_table.sgml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,9 +1265,9 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
12651265
<xref linkend="guc-default-tablespace"/> is consulted, or
12661266
<xref linkend="guc-temp-tablespaces"/> if the table is temporary. For
12671267
partitioned tables, since no storage is required for the table itself,
1268-
the tablespace specified here only serves to mark the default tablespace
1269-
for any newly created partitions when no other tablespace is explicitly
1270-
specified.
1268+
the tablespace specified overrides <literal>default_tablespace</literal>
1269+
as the default tablespace to use for any newly created partitions when no
1270+
other tablespace is explicitly specified.
12711271
</para>
12721272
</listitem>
12731273
</varlistentry>

‎src/backend/bootstrap/bootparse.y

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ Boot_DeclareIndexStmt:
314314
stmt->transformed = false;
315315
stmt->concurrent = false;
316316
stmt->if_not_exists = false;
317+
stmt->reset_default_tblspc = false;
317318

318319
/* locks and races need not concern us in bootstrap mode */
319320
relationId = RangeVarGetRelid(stmt->relation, NoLock,
@@ -363,6 +364,7 @@ Boot_DeclareUniqueIndexStmt:
363364
stmt->transformed = false;
364365
stmt->concurrent = false;
365366
stmt->if_not_exists = false;
367+
stmt->reset_default_tblspc = false;
366368

367369
/* locks and races need not concern us in bootstrap mode */
368370
relationId = RangeVarGetRelid(stmt->relation, NoLock,

‎src/backend/commands/indexcmds.c

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,21 @@ DefineIndex(Oid relationId,
467467
LOCKTAG heaplocktag;
468468
LOCKMODE lockmode;
469469
Snapshot snapshot;
470+
int save_nestlevel = -1;
470471
int i;
471472

473+
/*
474+
* Some callers need us to run with an empty default_tablespace; this is a
475+
* necessary hack to be able to reproduce catalog state accurately when
476+
* recreating indexes after table-rewriting ALTER TABLE.
477+
*/
478+
if (stmt->reset_default_tblspc)
479+
{
480+
save_nestlevel = NewGUCNestLevel();
481+
(void) set_config_option("default_tablespace", "",
482+
PGC_USERSET, PGC_S_SESSION,
483+
GUC_ACTION_SAVE, true, 0, false);
484+
}
472485

473486
/*
474487
* Start progress report. If we're building a partition, this was already
@@ -622,10 +635,15 @@ DefineIndex(Oid relationId,
622635
if (stmt->tableSpace)
623636
{
624637
tablespaceId = get_tablespace_oid(stmt->tableSpace, false);
638+
if (partitioned && tablespaceId == MyDatabaseTableSpace)
639+
ereport(ERROR,
640+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
641+
errmsg("cannot specify default tablespace for partitioned relation")));
625642
}
626643
else
627644
{
628-
tablespaceId = GetDefaultTablespace(rel->rd_rel->relpersistence);
645+
tablespaceId = GetDefaultTablespace(rel->rd_rel->relpersistence,
646+
partitioned);
629647
/* note InvalidOid is OK in this case */
630648
}
631649

@@ -980,6 +998,13 @@ DefineIndex(Oid relationId,
980998

981999
ObjectAddressSet(address, RelationRelationId, indexRelationId);
9821000

1001+
/*
1002+
* Revert to original default_tablespace. Must do this before any return
1003+
* from this function, but after index_create, so this is a good time.
1004+
*/
1005+
if (save_nestlevel >= 0)
1006+
AtEOXact_GUC(true, save_nestlevel);
1007+
9831008
if (!OidIsValid(indexRelationId))
9841009
{
9851010
table_close(rel, NoLock);

‎src/backend/commands/matview.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
284284
/* Concurrent refresh builds new data in temp tablespace, and does diff. */
285285
if (concurrent)
286286
{
287-
tableSpace = GetDefaultTablespace(RELPERSISTENCE_TEMP);
287+
tableSpace = GetDefaultTablespace(RELPERSISTENCE_TEMP, false);
288288
relpersistence = RELPERSISTENCE_TEMP;
289289
}
290290
else

‎src/backend/commands/tablecmds.c

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
567567
Datum reloptions;
568568
ListCell *listptr;
569569
AttrNumber attnum;
570+
bool partitioned;
570571
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
571572
Oid ofTypeId;
572573
ObjectAddress address;
@@ -595,7 +596,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
595596
elog(ERROR, "unexpected relkind: %d", (int) relkind);
596597

597598
relkind = RELKIND_PARTITIONED_TABLE;
599+
partitioned = true;
598600
}
601+
else
602+
partitioned = false;
599603

600604
/*
601605
* Look up the namespace in which we are supposed to create the relation,
@@ -664,31 +668,24 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
664668
if (stmt->tablespacename)
665669
{
666670
tablespaceId = get_tablespace_oid(stmt->tablespacename, false);
671+
672+
if (partitioned && tablespaceId == MyDatabaseTableSpace)
673+
ereport(ERROR,
674+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
675+
errmsg("cannot specify default tablespace for partitioned relations")));
667676
}
668677
else if (stmt->partbound)
669678
{
670-
HeapTuple tup;
671-
672679
/*
673680
* For partitions, when no other tablespace is specified, we default
674681
* the tablespace to the parent partitioned table's.
675682
*/
676683
Assert(list_length(inheritOids) == 1);
677-
tup = SearchSysCache1(RELOID,
678-
DatumGetObjectId(linitial_oid(inheritOids)));
679-
680-
tablespaceId = ((Form_pg_class) GETSTRUCT(tup))->reltablespace;
681-
682-
if (!OidIsValid(tablespaceId))
683-
tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence);
684-
685-
ReleaseSysCache(tup);
684+
tablespaceId = get_rel_tablespace(linitial_oid(inheritOids));
686685
}
687686
else
688-
{
689-
tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence);
690-
/* note InvalidOid is OK in this case */
691-
}
687+
tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence,
688+
partitioned);
692689

693690
/* Check permissions except when using database's default */
694691
if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
@@ -825,7 +822,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
825822
{
826823
accessMethod = stmt->accessMethod;
827824

828-
if (relkind == RELKIND_PARTITIONED_TABLE)
825+
if (partitioned)
829826
ereport(ERROR,
830827
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
831828
errmsg("specifying a table access method is not supported on a partitioned table")));
@@ -998,7 +995,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
998995
* Process the partitioning specification (if any) and store the partition
999996
* key information into the catalog.
1000997
*/
1001-
if (stmt->partspec)
998+
if (partitioned)
1002999
{
10031000
ParseState *pstate;
10041001
char strategy;
@@ -11276,6 +11273,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
1127611273

1127711274
if (!rewrite)
1127811275
TryReuseIndex(oldId, stmt);
11276+
stmt->reset_default_tblspc = true;
1127911277
/* keep the index's comment */
1128011278
stmt->idxcomment = GetComment(oldId, RelationRelationId, 0);
1128111279

@@ -11307,6 +11305,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
1130711305
/* keep any comment on the index */
1130811306
indstmt->idxcomment = GetComment(indoid,
1130911307
RelationRelationId, 0);
11308+
indstmt->reset_default_tblspc = true;
1131011309

1131111310
cmd->subtype = AT_ReAddIndex;
1131211311
tab->subcmds[AT_PASS_OLD_INDEX] =
@@ -11329,6 +11328,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
1132911328
if (con->contype == CONSTR_FOREIGN &&
1133011329
!rewrite && tab->rewrite == 0)
1133111330
TryReuseForeignKey(oldId, con);
11331+
con->reset_default_tblspc = true;
1133211332
cmd->subtype = AT_ReAddConstraint;
1133311333
tab->subcmds[AT_PASS_OLD_CONSTR] =
1133411334
lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd);

‎src/backend/commands/tablespace.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,7 +1104,9 @@ check_default_tablespace(char **newval, void **extra, GucSource source)
11041104
* GetDefaultTablespace -- get the OID of the current default tablespace
11051105
*
11061106
* Temporary objects have different default tablespaces, hence the
1107-
* relpersistence parameter must be specified.
1107+
* relpersistence parameter must be specified. Also, for partitioned tables,
1108+
* we disallow specifying the database default, so that needs to be specified
1109+
* too.
11081110
*
11091111
* May return InvalidOid to indicate "use the database's default tablespace".
11101112
*
@@ -1115,7 +1117,7 @@ check_default_tablespace(char **newval, void **extra, GucSource source)
11151117
* default_tablespace GUC variable.
11161118
*/
11171119
Oid
1118-
GetDefaultTablespace(char relpersistence)
1120+
GetDefaultTablespace(char relpersistence, bool partitioned)
11191121
{
11201122
Oid result;
11211123

@@ -1141,10 +1143,18 @@ GetDefaultTablespace(char relpersistence)
11411143

11421144
/*
11431145
* Allow explicit specification of database's default tablespace in
1144-
* default_tablespace without triggering permissions checks.
1146+
* default_tablespace without triggering permissions checks. Don't
1147+
* allow specifying that when creating a partitioned table, however,
1148+
* since the result is confusing.
11451149
*/
11461150
if (result == MyDatabaseTableSpace)
1151+
{
1152+
if (partitioned)
1153+
ereport(ERROR,
1154+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1155+
errmsg("cannot specify default tablespace for partitioned relations")));
11471156
result = InvalidOid;
1157+
}
11481158
return result;
11491159
}
11501160

‎src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2919,6 +2919,7 @@ _copyConstraint(const Constraint *from)
29192919
COPY_NODE_FIELD(options);
29202920
COPY_STRING_FIELD(indexname);
29212921
COPY_STRING_FIELD(indexspace);
2922+
COPY_SCALAR_FIELD(reset_default_tblspc);
29222923
COPY_STRING_FIELD(access_method);
29232924
COPY_NODE_FIELD(where_clause);
29242925
COPY_NODE_FIELD(pktable);
@@ -3475,6 +3476,7 @@ _copyIndexStmt(const IndexStmt *from)
34753476
COPY_SCALAR_FIELD(transformed);
34763477
COPY_SCALAR_FIELD(concurrent);
34773478
COPY_SCALAR_FIELD(if_not_exists);
3479+
COPY_SCALAR_FIELD(reset_default_tblspc);
34783480

34793481
return newnode;
34803482
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,7 @@ _equalIndexStmt(const IndexStmt *a, const IndexStmt *b)
13471347
COMPARE_SCALAR_FIELD(transformed);
13481348
COMPARE_SCALAR_FIELD(concurrent);
13491349
COMPARE_SCALAR_FIELD(if_not_exists);
1350+
COMPARE_SCALAR_FIELD(reset_default_tblspc);
13501351

13511352
return true;
13521353
}
@@ -2593,6 +2594,7 @@ _equalConstraint(const Constraint *a, const Constraint *b)
25932594
COMPARE_NODE_FIELD(options);
25942595
COMPARE_STRING_FIELD(indexname);
25952596
COMPARE_STRING_FIELD(indexspace);
2597+
COMPARE_SCALAR_FIELD(reset_default_tblspc);
25962598
COMPARE_STRING_FIELD(access_method);
25972599
COMPARE_NODE_FIELD(where_clause);
25982600
COMPARE_NODE_FIELD(pktable);

‎src/backend/nodes/outfuncs.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,6 +2649,7 @@ _outIndexStmt(StringInfo str, const IndexStmt *node)
26492649
WRITE_BOOL_FIELD(transformed);
26502650
WRITE_BOOL_FIELD(concurrent);
26512651
WRITE_BOOL_FIELD(if_not_exists);
2652+
WRITE_BOOL_FIELD(reset_default_tblspc);
26522653
}
26532654

26542655
static void
@@ -3491,6 +3492,7 @@ _outConstraint(StringInfo str, const Constraint *node)
34913492
WRITE_NODE_FIELD(options);
34923493
WRITE_STRING_FIELD(indexname);
34933494
WRITE_STRING_FIELD(indexspace);
3495+
WRITE_BOOL_FIELD(reset_default_tblspc);
34943496
/* access_method and where_clause not currently used */
34953497
break;
34963498

@@ -3501,6 +3503,7 @@ _outConstraint(StringInfo str, const Constraint *node)
35013503
WRITE_NODE_FIELD(options);
35023504
WRITE_STRING_FIELD(indexname);
35033505
WRITE_STRING_FIELD(indexspace);
3506+
WRITE_BOOL_FIELD(reset_default_tblspc);
35043507
/* access_method and where_clause not currently used */
35053508
break;
35063509

@@ -3511,6 +3514,7 @@ _outConstraint(StringInfo str, const Constraint *node)
35113514
WRITE_NODE_FIELD(options);
35123515
WRITE_STRING_FIELD(indexname);
35133516
WRITE_STRING_FIELD(indexspace);
3517+
WRITE_BOOL_FIELD(reset_default_tblspc);
35143518
WRITE_STRING_FIELD(access_method);
35153519
WRITE_NODE_FIELD(where_clause);
35163520
break;

‎src/backend/parser/gram.y

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7363,6 +7363,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name
73637363
n->initdeferred = false;
73647364
n->transformed = false;
73657365
n->if_not_exists = false;
7366+
n->reset_default_tblspc = false;
73667367
$$ = (Node *)n;
73677368
}
73687369
| CREATE opt_unique INDEX opt_concurrently IF_P NOT EXISTS index_name
@@ -7390,6 +7391,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name
73907391
n->initdeferred = false;
73917392
n->transformed = false;
73927393
n->if_not_exists = true;
7394+
n->reset_default_tblspc = false;
73937395
$$ = (Node *)n;
73947396
}
73957397
;

‎src/backend/parser/parse_utilcmd.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
14061406
index->transformed = true; /* don't need transformIndexStmt */
14071407
index->concurrent = false;
14081408
index->if_not_exists = false;
1409+
index->reset_default_tblspc = false;
14091410

14101411
/*
14111412
* We don't try to preserve the name of the source index; instead, just
@@ -2001,6 +2002,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
20012002
index->transformed = false;
20022003
index->concurrent = false;
20032004
index->if_not_exists = false;
2005+
index->reset_default_tblspc = constraint->reset_default_tblspc;
20042006

20052007
/*
20062008
* If it's ALTER TABLE ADD CONSTRAINT USING INDEX, look up the index and

‎src/backend/utils/adt/ruleutils.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,12 +1429,13 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
14291429
Oid tblspc;
14301430

14311431
tblspc = get_rel_tablespace(indexrelid);
1432-
if (!OidIsValid(tblspc))
1433-
tblspc = MyDatabaseTableSpace;
1434-
if (isConstraint)
1435-
appendStringInfoString(&buf, " USING INDEX");
1436-
appendStringInfo(&buf, " TABLESPACE %s",
1437-
quote_identifier(get_tablespace_name(tblspc)));
1432+
if (OidIsValid(tblspc))
1433+
{
1434+
if (isConstraint)
1435+
appendStringInfoString(&buf, " USING INDEX");
1436+
appendStringInfo(&buf, " TABLESPACE %s",
1437+
quote_identifier(get_tablespace_name(tblspc)));
1438+
}
14381439
}
14391440

14401441
/*
@@ -2170,6 +2171,12 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
21702171
pfree(options);
21712172
}
21722173

2174+
/*
2175+
* Print the tablespace, unless it's the database default.
2176+
* This is to help ALTER TABLE usage of this facility,
2177+
* which needs this behavior to recreate exact catalog
2178+
* state.
2179+
*/
21732180
tblspc = get_rel_tablespace(indexId);
21742181
if (OidIsValid(tblspc))
21752182
appendStringInfo(&buf, " USING INDEX TABLESPACE %s",

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 201904072
56+
#define CATALOG_VERSION_NO 201904251
5757

5858
#endif

‎src/include/commands/tablespace.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ extern Oid AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt);
4949

5050
extern void TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo);
5151

52-
extern Oid GetDefaultTablespace(char relpersistence);
52+
extern Oid GetDefaultTablespace(char relpersistence, bool partitioned);
5353

5454
extern void PrepareTempTablespaces(void);
5555

0 commit comments

Comments
 (0)