On Thu, Feb 04, 2021 at 03:38:39PM +0900, Michael Paquier wrote: > On Wed, Feb 03, 2021 at 07:54:42PM +0900, Michael Paquier wrote: > > Not sure I like that. Here is a proposal: > > "it is recommended to separately use ALTER TABLE ONLY on them so as > > any new partitions attached inherit the new tablespace value." > > So, I have done more work on this stuff today, and applied that as of > c5b2860.
> A second thing I have come back to is allow_system_table_mods for > toast relations, and decided to just forbid TABLESPACE if attempting > to use it directly on a system table even if allow_system_table_mods > is true. This was leading to inconsistent behaviors and weirdness in > the concurrent case because all the indexes are processed in series > after building a list. As we want to ignore the move of toast indexes > when moving the indexes of the parent table, this was leading to extra > conditions that are not really worth supporting after thinking about > it. One other issue was the lack of consistency when using pg_global > that was a no-op for the concurrent case but failed in the > non-concurrent case. I have put in place more regression tests for > all that. Isn't this dead code ? postgres=# REINDEX (CONCURRENTLY, TABLESPACE pg_global) TABLE pg_class; ERROR: 0A000: cannot reindex system catalogs concurrently LOCATION: ReindexRelationConcurrently, indexcmds.c:3276 diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 127ba7835d..c77a9b2563 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3260,73 +3260,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) { if (IsCatalogRelationOid(relationOid)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); ... - if (OidIsValid(params->tablespaceOid) && - IsSystemRelation(heapRelation)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(heapRelation)))); - @@ -3404,73 +3397,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) if (IsCatalogRelationOid(heapId)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); ... - if (OidIsValid(params->tablespaceOid) && - IsSystemRelation(heapRelation)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - get_rel_name(relationOid)))); -
>From b4347c18bc732d30295c065ef71edaac65e68fe6 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sun, 14 Feb 2021 19:49:52 -0600 Subject: [PATCH] Dead code: REINDEX (CONCURRENTLY, TABLESPACE ..): c5b286047cd698021e57a527215b48865fd4ad4e --- src/backend/commands/indexcmds.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 127ba7835d..c77a9b2563 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3260,73 +3260,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) { /* * In the case of a relation, find all its indexes including * toast indexes. */ Relation heapRelation; /* Save the list of relation OIDs in private context */ oldcontext = MemoryContextSwitchTo(private_context); /* Track this relation for session locks */ heapRelationIds = lappend_oid(heapRelationIds, relationOid); MemoryContextSwitchTo(oldcontext); if (IsCatalogRelationOid(relationOid)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); /* Open relation to get its indexes */ if ((params->options & REINDEXOPT_MISSING_OK) != 0) { heapRelation = try_table_open(relationOid, ShareUpdateExclusiveLock); /* leave if relation does not exist */ if (!heapRelation) break; } else heapRelation = table_open(relationOid, ShareUpdateExclusiveLock); - if (OidIsValid(params->tablespaceOid) && - IsSystemRelation(heapRelation)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(heapRelation)))); - /* Add all the valid indexes of relation to list */ foreach(lc, RelationGetIndexList(heapRelation)) { Oid cellOid = lfirst_oid(lc); Relation indexRelation = index_open(cellOid, ShareUpdateExclusiveLock); if (!indexRelation->rd_index->indisvalid) ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex invalid index \"%s.%s\" concurrently, skipping", get_namespace_name(get_rel_namespace(cellOid)), get_rel_name(cellOid)))); else if (indexRelation->rd_index->indisexclusion) ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex exclusion constraint index \"%s.%s\" concurrently, skipping", get_namespace_name(get_rel_namespace(cellOid)), get_rel_name(cellOid)))); else { ReindexIndexInfo *idx; /* Save the list of relation OIDs in private context */ oldcontext = MemoryContextSwitchTo(private_context); idx = palloc(sizeof(ReindexIndexInfo)); idx->indexId = cellOid; /* other fields set later */ indexIds = lappend(indexIds, idx); MemoryContextSwitchTo(oldcontext); @@ -3404,73 +3397,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); /* * Don't allow reindex for an invalid index on TOAST table, as * if rebuilt it would not be possible to drop it. Match * error message in reindex_index(). */ if (IsToastNamespace(get_rel_namespace(relationOid)) && !get_index_isvalid(relationOid)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex invalid index on TOAST table"))); /* * Check if parent relation can be locked and if it exists, * this needs to be done at this stage as the list of indexes * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK * should not be used once all the session locks are taken. */ if ((params->options & REINDEXOPT_MISSING_OK) != 0) { heapRelation = try_table_open(heapId, ShareUpdateExclusiveLock); /* leave if relation does not exist */ if (!heapRelation) break; } else heapRelation = table_open(heapId, ShareUpdateExclusiveLock); - if (OidIsValid(params->tablespaceOid) && - IsSystemRelation(heapRelation)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - get_rel_name(relationOid)))); - table_close(heapRelation, NoLock); /* Save the list of relation OIDs in private context */ oldcontext = MemoryContextSwitchTo(private_context); /* Track the heap relation of this index for session locks */ heapRelationIds = list_make1_oid(heapId); /* * Save the list of relation OIDs in private context. Note * that invalid indexes are allowed here. */ idx = palloc(sizeof(ReindexIndexInfo)); idx->indexId = relationOid; indexIds = lappend(indexIds, idx); /* other fields set later */ MemoryContextSwitchTo(oldcontext); break; } case RELKIND_PARTITIONED_TABLE: case RELKIND_PARTITIONED_INDEX: default: /* Return error if type of relation is not supported */ ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot reindex this type of relation concurrently"))); break; } /* * Definitely no indexes, so leave. Any checks based on -- 2.17.0