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 <[email protected]>
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
