agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Steve Singer <steve@ssinger.info>
Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Jose Luis Tallon <jltallon@adv-solutions.net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: Tue, 22 Dec 2020 15:15:37 -0600
Message-ID: <20201222211537.GO30237@telsasoft.com> (raw)
In-Reply-To: <X+HDFVXTYbTJMSXZ@paquier.xyz>
References: <7ec67c56-2377-cd05-51a0-691104404abe@enterprisedb.com>
<20201216004517.GA18498@alvherre.pgsql>
<X9lcV/nYVU5WYf7D@paquier.xyz>
<X+GWnexHZdt/aaVl@paquier.xyz>
<20201222083204.GL30237@telsasoft.com>
<X+HDFVXTYbTJMSXZ@paquier.xyz>
On Tue, Dec 22, 2020 at 06:57:41PM +0900, Michael Paquier wrote:
> On Tue, Dec 22, 2020 at 02:32:05AM -0600, Justin Pryzby wrote:
> > Also, this one is going to be subsumed by ExecReindex(), so the palloc will go
> > away (otherwise I would ask to pass it in from the caller):
>
> Yeah, maybe. Still you need to be very careful if you have any
> allocated variables like a tablespace or a path which requires to be
> in the private context used by ReindexMultipleInternal() or even
> ReindexRelationConcurrently(), so I am not sure you can avoid that
> completely. For now, we could choose the option to still use a
> palloc(), and then save the options in the private contexts. Forgot
> that in the previous version actually.
I can't see why this still uses memset instead of structure assignment.
Now, I really think utility.c ought to pass in a pointer to a local
ReindexOptions variable to avoid all the memory context, which is unnecessary
and prone to error.
ExecReindex() will set options.tablesapceOid, not a pointer. Like this.
I also changed the callback to be a ReindexOptions and not a pointer.
--
Justin
Attachments:
[text/x-diff] 0001-refactor-utility-opts-michael-2.patch-hacked-on-by-J.patch (26.5K, ../20201222211537.GO30237@telsasoft.com/2-0001-refactor-utility-opts-michael-2.patch-hacked-on-by-J.patch)
download | inline diff:
From f12723a8e4ad550c009935fa5397d1d4a968c7bf Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 22 Dec 2020 18:57:41 +0900
Subject: [PATCH 1/3] refactor-utility-opts-michael-2.patch - hacked on by
Justin
---
src/backend/catalog/index.c | 19 ++++---
src/backend/commands/cluster.c | 19 ++++---
src/backend/commands/indexcmds.c | 97 +++++++++++++++++---------------
src/backend/commands/tablecmds.c | 3 +-
src/backend/commands/vacuum.c | 6 +-
src/backend/tcop/utility.c | 12 ++--
src/include/catalog/index.h | 19 ++++---
src/include/commands/cluster.h | 13 +++--
src/include/commands/defrem.h | 17 ++++--
src/include/commands/vacuum.h | 20 +++----
src/tools/pgindent/typedefs.list | 2 +
11 files changed, 124 insertions(+), 103 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 731610c701..d776c661d7 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3594,7 +3594,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
*/
void
reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
- int options)
+ ReindexOptions *options)
{
Relation iRel,
heapRelation;
@@ -3602,7 +3602,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
PGRUsage ru0;
- bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
+ bool progress = ((options->flags & REINDEXOPT_REPORT_PROGRESS) != 0);
pg_rusage_init(&ru0);
@@ -3611,12 +3611,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* we only need to be sure no schema or data changes are going on.
*/
heapId = IndexGetRelation(indexId,
- (options & REINDEXOPT_MISSING_OK) != 0);
+ (options->flags & REINDEXOPT_MISSING_OK) != 0);
/* if relation is missing, leave */
if (!OidIsValid(heapId))
return;
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if ((options->flags & REINDEXOPT_MISSING_OK) != 0)
heapRelation = try_table_open(heapId, ShareLock);
else
heapRelation = table_open(heapId, ShareLock);
@@ -3792,7 +3792,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
}
/* Log what we did */
- if (options & REINDEXOPT_VERBOSE)
+ if ((options->flags & REINDEXOPT_VERBOSE) != 0)
ereport(INFO,
(errmsg("index \"%s\" was reindexed",
get_rel_name(indexId)),
@@ -3846,7 +3846,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* index rebuild.
*/
bool
-reindex_relation(Oid relid, int flags, int options)
+reindex_relation(Oid relid, int flags, ReindexOptions *options)
{
Relation rel;
Oid toast_relid;
@@ -3861,7 +3861,7 @@ reindex_relation(Oid relid, int flags, int options)
* to prevent schema and data changes in it. The lock level used here
* should match ReindexTable().
*/
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if ((options->flags & REINDEXOPT_MISSING_OK) != 0)
rel = try_table_open(relid, ShareLock);
else
rel = table_open(relid, ShareLock);
@@ -3965,8 +3965,9 @@ reindex_relation(Oid relid, int flags, int options)
* Note that this should fail if the toast relation is missing, so
* reset REINDEXOPT_MISSING_OK.
*/
- result |= reindex_relation(toast_relid, flags,
- options & ~(REINDEXOPT_MISSING_OK));
+ ReindexOptions newoptions = *options;
+ newoptions.flags &= ~(REINDEXOPT_MISSING_OK);
+ result |= reindex_relation(toast_relid, flags, &newoptions);
}
return result;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index fd5a6eec86..32f5ae848f 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -103,7 +103,7 @@ void
cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
{
ListCell *lc;
- int options = 0;
+ ClusterOptions options = {0};
bool verbose = false;
/* Parse option list */
@@ -121,7 +121,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
parser_errposition(pstate, opt->location)));
}
- options = (verbose ? CLUOPT_VERBOSE : 0);
+ options.flags = (verbose ? CLUOPT_VERBOSE : 0);
if (stmt->relation != NULL)
{
@@ -192,7 +192,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
table_close(rel, NoLock);
/* Do the job. */
- cluster_rel(tableOid, indexOid, options);
+ cluster_rel(tableOid, indexOid, &options);
}
else
{
@@ -234,6 +234,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
foreach(rv, rvs)
{
RelToCluster *rvtc = (RelToCluster *) lfirst(rv);
+ ClusterOptions cluster_options = options;
+ cluster_options.flags |= CLUOPT_RECHECK;
/* Start a new transaction for each relation. */
StartTransactionCommand();
@@ -241,7 +243,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
PushActiveSnapshot(GetTransactionSnapshot());
/* Do the job. */
cluster_rel(rvtc->tableOid, rvtc->indexOid,
- options | CLUOPT_RECHECK);
+ &cluster_options);
PopActiveSnapshot();
CommitTransactionCommand();
}
@@ -272,11 +274,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
* and error messages should refer to the operation as VACUUM not CLUSTER.
*/
void
-cluster_rel(Oid tableOid, Oid indexOid, int options)
+cluster_rel(Oid tableOid, Oid indexOid, ClusterOptions *options)
{
Relation OldHeap;
- bool verbose = ((options & CLUOPT_VERBOSE) != 0);
- bool recheck = ((options & CLUOPT_RECHECK) != 0);
+ bool verbose = ((options->flags & CLUOPT_VERBOSE) != 0);
+ bool recheck = ((options->flags & CLUOPT_RECHECK) != 0);
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
@@ -1355,6 +1357,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
ObjectAddress object;
Oid mapped_tables[4];
int reindex_flags;
+ ReindexOptions reindex_options = {0};
int i;
/* Report that we are now swapping relation files */
@@ -1412,7 +1415,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
- reindex_relation(OIDOldHeap, reindex_flags, 0);
+ reindex_relation(OIDOldHeap, reindex_flags, &reindex_options);
/* Report that we are now doing clean up */
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 14d24b3cc4..98a8ade20b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -89,9 +89,12 @@ static List *ChooseIndexColumnNames(List *indexElems);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static void reindex_error_callback(void *args);
-static void ReindexPartitions(Oid relid, int options, bool isTopLevel);
-static void ReindexMultipleInternal(List *relids, int options);
-static bool ReindexRelationConcurrently(Oid relationOid, int options);
+static void ReindexPartitions(Oid relid, ReindexOptions *options,
+ bool isTopLevel);
+static void ReindexMultipleInternal(List *relids,
+ ReindexOptions *options);
+static bool ReindexRelationConcurrently(Oid relationOid,
+ ReindexOptions *options);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
@@ -100,7 +103,7 @@ static inline void set_indexsafe_procflags(void);
*/
struct ReindexIndexCallbackState
{
- int options; /* options from statement */
+ ReindexOptions options; /* options from statement */
Oid locked_table_oid; /* tracks previously locked table */
};
@@ -2453,13 +2456,12 @@ ChooseIndexColumnNames(List *indexElems)
/*
* ReindexParseOptions
- * Parse list of REINDEX options, returning a bitmask of ReindexOption.
+ * Parse stmt into options.
*/
-int
-ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
+void
+ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt, ReindexOptions *options)
{
ListCell *lc;
- int options = 0;
bool concurrently = false;
bool verbose = false;
@@ -2480,11 +2482,9 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
parser_errposition(pstate, opt->location)));
}
- options =
+ options->flags =
(verbose ? REINDEXOPT_VERBOSE : 0) |
(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
-
- return options;
}
/*
@@ -2492,7 +2492,7 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
* Recreate a specific index.
*/
void
-ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
+ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
Oid indOid;
@@ -2509,10 +2509,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
* upgrade the lock, but that's OK, because other sessions can't hold
* locks on our temporary table.
*/
- state.options = options;
+ state.options = *options;
state.locked_table_oid = InvalidOid;
indOid = RangeVarGetRelidExtended(indexRelation,
- (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ (options->flags & REINDEXOPT_CONCURRENTLY) != 0 ?
ShareUpdateExclusiveLock : AccessExclusiveLock,
0,
RangeVarCallbackForReindexIndex,
@@ -2527,12 +2527,15 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
if (relkind == RELKIND_PARTITIONED_INDEX)
ReindexPartitions(indOid, options, isTopLevel);
- else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ else if ((options->flags & REINDEXOPT_CONCURRENTLY) != 0 &&
persistence != RELPERSISTENCE_TEMP)
ReindexRelationConcurrently(indOid, options);
else
- reindex_index(indOid, false, persistence,
- options | REINDEXOPT_REPORT_PROGRESS);
+ {
+ ReindexOptions newoptions = *options;
+ newoptions.flags |= REINDEXOPT_REPORT_PROGRESS;
+ reindex_index(indOid, false, persistence, &newoptions);
+ }
}
/*
@@ -2553,7 +2556,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* non-concurrent case and table locks used by index_concurrently_*() for
* concurrent case.
*/
- table_lockmode = ((state->options & REINDEXOPT_CONCURRENTLY) != 0) ?
+ table_lockmode = (state->options.flags & REINDEXOPT_CONCURRENTLY) != 0 ?
ShareUpdateExclusiveLock : ShareLock;
/*
@@ -2611,7 +2614,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation, int options, bool isTopLevel)
+ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
{
Oid heapOid;
bool result;
@@ -2625,14 +2628,14 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
* locks on our temporary table.
*/
heapOid = RangeVarGetRelidExtended(relation,
- (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ (options->flags & REINDEXOPT_CONCURRENTLY) != 0 ?
ShareUpdateExclusiveLock : ShareLock,
0,
RangeVarCallbackOwnsTable, NULL);
if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
ReindexPartitions(heapOid, options, isTopLevel);
- else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ else if ((options->flags & REINDEXOPT_CONCURRENTLY) != 0 &&
get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
result = ReindexRelationConcurrently(heapOid, options);
@@ -2644,10 +2647,12 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
}
else
{
+ ReindexOptions newoptions = *options;
+ newoptions.flags |= REINDEXOPT_REPORT_PROGRESS;
result = reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options | REINDEXOPT_REPORT_PROGRESS);
+ &newoptions);
if (!result)
ereport(NOTICE,
(errmsg("table \"%s\" has no indexes to reindex",
@@ -2667,7 +2672,7 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
*/
void
ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options)
+ ReindexOptions *options)
{
Oid objectOid;
Relation relationRelation;
@@ -2686,7 +2691,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
objectKind == REINDEX_OBJECT_DATABASE);
if (objectKind == REINDEX_OBJECT_SYSTEM &&
- (options & REINDEXOPT_CONCURRENTLY) != 0)
+ (options->flags & REINDEXOPT_CONCURRENTLY) != 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
@@ -2794,7 +2799,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Skip system tables, since index_create() would reject indexing them
* concurrently (and it would likely fail if we tried).
*/
- if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ if ((options->flags & REINDEXOPT_CONCURRENTLY) != 0 &&
IsCatalogRelationOid(relid))
{
if (!concurrent_warning)
@@ -2860,7 +2865,7 @@ reindex_error_callback(void *arg)
* by the caller.
*/
static void
-ReindexPartitions(Oid relid, int options, bool isTopLevel)
+ReindexPartitions(Oid relid, ReindexOptions *options, bool isTopLevel)
{
List *partitions = NIL;
char relkind = get_rel_relkind(relid);
@@ -2955,7 +2960,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel)
* and starts a new transaction when finished.
*/
static void
-ReindexMultipleInternal(List *relids, int options)
+ReindexMultipleInternal(List *relids, ReindexOptions *options)
{
ListCell *l;
@@ -2991,35 +2996,35 @@ ReindexMultipleInternal(List *relids, int options)
Assert(relkind != RELKIND_PARTITIONED_INDEX &&
relkind != RELKIND_PARTITIONED_TABLE);
- if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ if ((options->flags & REINDEXOPT_CONCURRENTLY) != 0 &&
relpersistence != RELPERSISTENCE_TEMP)
{
- (void) ReindexRelationConcurrently(relid,
- options |
- REINDEXOPT_MISSING_OK);
+ ReindexOptions newoptions = *options;
+ newoptions.flags |= REINDEXOPT_MISSING_OK;
+ (void) ReindexRelationConcurrently(relid, &newoptions);
/* ReindexRelationConcurrently() does the verbose output */
}
else if (relkind == RELKIND_INDEX)
{
- reindex_index(relid, false, relpersistence,
- options |
- REINDEXOPT_REPORT_PROGRESS |
- REINDEXOPT_MISSING_OK);
+ ReindexOptions newoptions = *options;
+ newoptions.flags |=
+ REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
+ reindex_index(relid, false, relpersistence, &newoptions);
PopActiveSnapshot();
/* reindex_index() does the verbose output */
}
else
{
bool result;
-
+ ReindexOptions newoptions = *options;
+ newoptions.flags |=
+ REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options |
- REINDEXOPT_REPORT_PROGRESS |
- REINDEXOPT_MISSING_OK);
+ &newoptions);
- if (result && (options & REINDEXOPT_VERBOSE))
+ if (result && (options->flags & REINDEXOPT_VERBOSE) != 0)
ereport(INFO,
(errmsg("table \"%s.%s\" was reindexed",
get_namespace_name(get_rel_namespace(relid)),
@@ -3059,7 +3064,7 @@ ReindexMultipleInternal(List *relids, int options)
* anyway, and a non-concurrent reindex is more efficient.
*/
static bool
-ReindexRelationConcurrently(Oid relationOid, int options)
+ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options)
{
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3092,7 +3097,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
"ReindexConcurrent",
ALLOCSET_SMALL_SIZES);
- if (options & REINDEXOPT_VERBOSE)
+ if ((options->flags & REINDEXOPT_VERBOSE) != 0)
{
/* Save data needed by REINDEX VERBOSE in private context */
oldcontext = MemoryContextSwitchTo(private_context);
@@ -3137,7 +3142,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
errmsg("cannot reindex system catalogs concurrently")));
/* Open relation to get its indexes */
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if ((options->flags & REINDEXOPT_MISSING_OK) != 0)
{
heapRelation = try_table_open(relationOid,
ShareUpdateExclusiveLock);
@@ -3233,7 +3238,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
case RELKIND_INDEX:
{
Oid heapId = IndexGetRelation(relationOid,
- (options & REINDEXOPT_MISSING_OK) != 0);
+ (options->flags & REINDEXOPT_MISSING_OK) != 0);
Relation heapRelation;
/* if relation is missing, leave */
@@ -3262,7 +3267,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
* should not be used once all the session locks are taken.
*/
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if ((options->flags & REINDEXOPT_MISSING_OK) != 0)
{
heapRelation = try_table_open(heapId,
ShareUpdateExclusiveLock);
@@ -3754,7 +3759,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
/* Log what we did */
- if (options & REINDEXOPT_VERBOSE)
+ if ((options->flags & REINDEXOPT_VERBOSE) != 0)
{
if (relkind == RELKIND_INDEX)
ereport(INFO,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1fa9f19f08..7d6b321d75 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1854,6 +1854,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
{
Oid heap_relid;
Oid toast_relid;
+ ReindexOptions reindex_options = {0};
/*
* This effectively deletes all rows in the table, and may be done
@@ -1891,7 +1892,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
/*
* Reconstruct the indexes to match, and we're done.
*/
- reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, 0);
+ reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, &reindex_options);
}
pgstat_count_truncate(rel);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 98270a1049..985450e8f8 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1916,17 +1916,17 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
*/
if (params->options & VACOPT_FULL)
{
- int cluster_options = 0;
+ ClusterOptions cluster_options = {0};
/* close relation before vacuuming, but hold lock until commit */
relation_close(onerel, NoLock);
onerel = NULL;
if ((params->options & VACOPT_VERBOSE) != 0)
- cluster_options |= CLUOPT_VERBOSE;
+ cluster_options.flags |= CLUOPT_VERBOSE;
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
- cluster_rel(relid, InvalidOid, cluster_options);
+ cluster_rel(relid, InvalidOid, &cluster_options);
}
else
table_relation_vacuum(onerel, params, vac_strategy);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index a42ead7d69..403b46cefb 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -919,20 +919,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
case T_ReindexStmt:
{
ReindexStmt *stmt = (ReindexStmt *) parsetree;
- int options;
+ ReindexOptions options;
- options = ReindexParseOptions(pstate, stmt);
- if ((options & REINDEXOPT_CONCURRENTLY) != 0)
+ ReindexParseOptions(pstate, stmt, &options);
+ if ((options.flags & REINDEXOPT_CONCURRENTLY) != 0)
PreventInTransactionBlock(isTopLevel,
"REINDEX CONCURRENTLY");
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, options, isTopLevel);
+ ReindexIndex(stmt->relation, &options, isTopLevel);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, options, isTopLevel);
+ ReindexTable(stmt->relation, &options, isTopLevel);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
@@ -948,7 +948,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
(stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
(stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
"REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind, options);
+ ReindexMultipleTables(stmt->name, stmt->kind, &options);
break;
default:
elog(ERROR, "unrecognized object type: %d",
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index c041628049..89394b648e 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -30,13 +30,16 @@ typedef enum
} IndexStateFlagsAction;
/* options for REINDEX */
-typedef enum ReindexOption
+typedef struct ReindexOptions
{
- REINDEXOPT_VERBOSE = 1 << 0, /* print progress info */
- REINDEXOPT_REPORT_PROGRESS = 1 << 1, /* report pgstat progress */
- REINDEXOPT_MISSING_OK = 1 << 2, /* skip missing relations */
- REINDEXOPT_CONCURRENTLY = 1 << 3 /* concurrent mode */
-} ReindexOption;
+ bits32 flags; /* bitmask of REINDEXOPT_* */
+} ReindexOptions;
+
+/* flag bits for ReindexOptions->flags */
+#define REINDEXOPT_VERBOSE 0x01 /* print progress info */
+#define REINDEXOPT_REPORT_PROGRESS 0x02 /* report pgstat progress */
+#define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */
+#define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
@@ -146,7 +149,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
extern Oid IndexGetRelation(Oid indexId, bool missing_ok);
extern void reindex_index(Oid indexId, bool skip_constraint_checks,
- char relpersistence, int options);
+ char relpersistence, ReindexOptions *options);
/* Flag bits for reindex_relation(): */
#define REINDEX_REL_PROCESS_TOAST 0x01
@@ -155,7 +158,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
#define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08
#define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
-extern bool reindex_relation(Oid relid, int flags, int options);
+extern bool reindex_relation(Oid relid, int flags, ReindexOptions *options);
extern bool ReindexIsProcessingHeap(Oid heapOid);
extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index 7cfb37c9b2..c66629cf73 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -18,16 +18,17 @@
#include "storage/lock.h"
#include "utils/relcache.h"
-
/* options for CLUSTER */
-typedef enum ClusterOption
+#define CLUOPT_RECHECK 0x01 /* recheck relation state */
+#define CLUOPT_VERBOSE 0x02 /* print progress info */
+
+typedef struct ClusterOptions
{
- CLUOPT_RECHECK = 1 << 0, /* recheck relation state */
- CLUOPT_VERBOSE = 1 << 1 /* print progress info */
-} ClusterOption;
+ bits32 flags; /* bitmask of CLUSTEROPT_* */
+} ClusterOptions;
extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel);
-extern void cluster_rel(Oid tableOid, Oid indexOid, int options);
+extern void cluster_rel(Oid tableOid, Oid indexOid, ClusterOptions *options);
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
bool recheck, LOCKMODE lockmode);
extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 1133ae1143..ca276e222d 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -14,6 +14,7 @@
#ifndef DEFREM_H
#define DEFREM_H
+#include "catalog/index.h"
#include "catalog/objectaddress.h"
#include "nodes/params.h"
#include "parser/parse_node.h"
@@ -34,11 +35,17 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern int ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel);
-extern Oid ReindexTable(RangeVar *relation, int options, bool isTopLevel);
-extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options);
+extern void ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt,
+ ReindexOptions *options);
+extern void ReindexIndex(RangeVar *indexRelation,
+ ReindexOptions *options,
+ bool isTopLevel);
+extern Oid ReindexTable(RangeVar *relation,
+ ReindexOptions *options,
+ bool isTopLevel);
+extern void ReindexMultipleTables(const char *objectName,
+ ReindexObjectType objectKind,
+ ReindexOptions *options);
extern char *makeObjectName(const char *name1, const char *name2,
const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2,
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index a4cd721400..e3fe4b38e8 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -174,17 +174,15 @@ typedef struct VacAttrStats
int rowstride;
} VacAttrStats;
-typedef enum VacuumOption
-{
- VACOPT_VACUUM = 1 << 0, /* do VACUUM */
- VACOPT_ANALYZE = 1 << 1, /* do ANALYZE */
- VACOPT_VERBOSE = 1 << 2, /* print progress info */
- VACOPT_FREEZE = 1 << 3, /* FREEZE option */
- VACOPT_FULL = 1 << 4, /* FULL (non-concurrent) vacuum */
- VACOPT_SKIP_LOCKED = 1 << 5, /* skip if cannot get lock */
- VACOPT_SKIPTOAST = 1 << 6, /* don't process the TOAST table, if any */
- VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7 /* don't skip any pages */
-} VacuumOption;
+/* flag bits for VacuumParams->options */
+#define VACOPT_VACUUM 0x01 /* do VACUUM */
+#define VACOPT_ANALYZE 0x02 /* do ANALYZE */
+#define VACOPT_VERBOSE 0x04 /* print progress info */
+#define VACOPT_FREEZE 0x08 /* FREEZE option */
+#define VACOPT_FULL 0x10 /* FULL (non-concurrent) vacuum */
+#define VACOPT_SKIP_LOCKED 0x20 /* skip if cannot get lock */
+#define VACOPT_SKIPTOAST 0x40 /* don't process the TOAST table, if any */
+#define VACOPT_DISABLE_PAGE_SKIPPING 0x80 /* don't skip any pages */
/*
* A ternary value used by vacuum parameters.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index bca37c536e..768f758fa0 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -347,6 +347,7 @@ ClosePortalStmt
ClosePtrType
Clump
ClusterInfo
+ClusterOptions
ClusterStmt
CmdType
CoalesceExpr
@@ -2050,6 +2051,7 @@ RegisNode
RegisteredBgWorker
ReindexErrorInfo
ReindexObjectType
+ReindexOptions
ReindexStmt
ReindexType
RelFileNode
--
2.17.0
[text/x-diff] 0002-ExecReindex.patch (7.0K, ../20201222211537.GO30237@telsasoft.com/3-0002-ExecReindex.patch)
download | inline diff:
From 03b235487cadd402130848430feabbb3a1abec1f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 22 Dec 2020 14:53:25 -0600
Subject: [PATCH 2/3] ExecReindex
---
src/backend/commands/indexcmds.c | 59 ++++++++++++++++++++++++++++----
src/backend/tcop/utility.c | 39 ++-------------------
src/include/commands/defrem.h | 12 +------
3 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 98a8ade20b..edd394ed4e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -86,8 +86,14 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
bool primary, bool isconstraint);
static char *ChooseIndexNameAddition(List *colnames);
static List *ChooseIndexColumnNames(List *indexElems);
+static void ReindexIndex(RangeVar *indexRelation, ReindexOptions *options,
+ bool isTopLevel);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
+static Oid ReindexTable(RangeVar *relation, ReindexOptions *options,
+ bool isTopLevel);
+static void ReindexMultipleTables(const char *objectName,
+ ReindexObjectType objectKind, ReindexOptions *options);
static void reindex_error_callback(void *args);
static void ReindexPartitions(Oid relid, ReindexOptions *options,
bool isTopLevel);
@@ -2455,12 +2461,17 @@ ChooseIndexColumnNames(List *indexElems)
}
/*
- * ReindexParseOptions
- * Parse stmt into options.
+ * ExecReindex
+ * Reindex accordinging to stmt.
+ * This calls the intermediate routines: ReindexIndex, ReindexTable, ReindexMultipleTables,
+ * which ultimately call reindex_index, reindex_relation, ReindexRelationConcurrently.
+ * Note that partitioned relations are handled by ReindexPartitions, except that
+ * ReindexRelationConcurrently handles concurrently reindexing a table.
*/
void
-ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt, ReindexOptions *options)
+ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
{
+ ReindexOptions options = {0};
ListCell *lc;
bool concurrently = false;
bool verbose = false;
@@ -2482,16 +2493,50 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt, ReindexOptions *optio
parser_errposition(pstate, opt->location)));
}
- options->flags =
+ if (concurrently)
+ PreventInTransactionBlock(isTopLevel,
+ "REINDEX CONCURRENTLY");
+
+ options.flags =
(verbose ? REINDEXOPT_VERBOSE : 0) |
(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
+
+ switch (stmt->kind)
+ {
+ case REINDEX_OBJECT_INDEX:
+ ReindexIndex(stmt->relation, &options, isTopLevel);
+ break;
+ case REINDEX_OBJECT_TABLE:
+ ReindexTable(stmt->relation, &options, isTopLevel);
+ break;
+ case REINDEX_OBJECT_SCHEMA:
+ case REINDEX_OBJECT_SYSTEM:
+ case REINDEX_OBJECT_DATABASE:
+
+ /*
+ * This cannot run inside a user transaction block; if
+ * we were inside a transaction, then its commit- and
+ * start-transaction-command calls would not have the
+ * intended effect!
+ */
+ PreventInTransactionBlock(isTopLevel,
+ (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
+ (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
+ "REINDEX DATABASE");
+ ReindexMultipleTables(stmt->name, stmt->kind, &options);
+ break;
+ default:
+ elog(ERROR, "unrecognized object type: %d",
+ (int) stmt->kind);
+ break;
+ }
}
/*
* ReindexIndex
* Recreate a specific index.
*/
-void
+static void
ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
@@ -2613,7 +2658,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* ReindexTable
* Recreate all indexes of a table (and of its toast table, if any)
*/
-Oid
+static Oid
ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
{
Oid heapOid;
@@ -2670,7 +2715,7 @@ ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
* separate transaction, so we can release the lock on it right away.
* That means this must not be called within a user transaction block!
*/
-void
+static void
ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
ReindexOptions *options)
{
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 403b46cefb..4942e24a3a 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -919,44 +919,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
case T_ReindexStmt:
{
ReindexStmt *stmt = (ReindexStmt *) parsetree;
- ReindexOptions options;
-
- ReindexParseOptions(pstate, stmt, &options);
- if ((options.flags & REINDEXOPT_CONCURRENTLY) != 0)
- PreventInTransactionBlock(isTopLevel,
- "REINDEX CONCURRENTLY");
-
- switch (stmt->kind)
- {
- case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, &options, isTopLevel);
- break;
- case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, &options, isTopLevel);
- break;
- case REINDEX_OBJECT_SCHEMA:
- case REINDEX_OBJECT_SYSTEM:
- case REINDEX_OBJECT_DATABASE:
-
- /*
- * This cannot run inside a user transaction block; if
- * we were inside a transaction, then its commit- and
- * start-transaction-command calls would not have the
- * intended effect!
- */
- PreventInTransactionBlock(isTopLevel,
- (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
- (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
- "REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind, &options);
- break;
- default:
- elog(ERROR, "unrecognized object type: %d",
- (int) stmt->kind);
- break;
- }
+ ExecReindex(pstate, stmt, isTopLevel);
+ break;
}
- break;
/*
* The following statements are supported by Event Triggers only
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index ca276e222d..d4ea57e757 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -35,17 +35,7 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern void ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt,
- ReindexOptions *options);
-extern void ReindexIndex(RangeVar *indexRelation,
- ReindexOptions *options,
- bool isTopLevel);
-extern Oid ReindexTable(RangeVar *relation,
- ReindexOptions *options,
- bool isTopLevel);
-extern void ReindexMultipleTables(const char *objectName,
- ReindexObjectType objectKind,
- ReindexOptions *options);
+extern void ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel);
extern char *makeObjectName(const char *name1, const char *name2,
const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2,
--
2.17.0
[text/x-diff] 0003-Allow-REINDEX-to-change-tablespace.patch (24.8K, ../20201222211537.GO30237@telsasoft.com/4-0003-Allow-REINDEX-to-change-tablespace.patch)
download | inline diff:
From 899ac88c3310bd33052a8c29564ec5d2f1b6725f Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.aleksey@gmail.com>
Date: Mon, 23 Mar 2020 21:10:29 +0300
Subject: [PATCH 3/3] Allow REINDEX to change tablespace
REINDEX already does full relation rewrite, this patch adds a
possibility to specify a new tablespace where new relfilenode
will be created.
---
doc/src/sgml/ref/reindex.sgml | 22 +++++
src/backend/catalog/index.c | 96 +++++++++++++++++++-
src/backend/commands/indexcmds.c | 103 +++++++++++++++++++++-
src/bin/psql/tab-complete.c | 4 +-
src/include/catalog/index.h | 2 +
src/test/regress/input/tablespace.source | 53 +++++++++++
src/test/regress/output/tablespace.source | 102 +++++++++++++++++++++
7 files changed, 377 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 6e1cf06713..e58b1281cd 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -27,6 +27,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
CONCURRENTLY [ <replaceable class="parameter">boolean</replaceable> ]
VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
+ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -187,6 +188,19 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLESPACE</literal></term>
+ <listitem>
+ <para>
+ This specifies that indexes will be rebuilt on a new tablespace.
+ Cannot be used with "mapped" relations. If <literal>SCHEMA</literal>,
+ <literal>DATABASE</literal> or <literal>SYSTEM</literal> is specified, then
+ all unsuitable relations will be skipped and a single <literal>WARNING</literal>
+ will be generated.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>VERBOSE</literal></term>
<listitem>
@@ -210,6 +224,14 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_tablespace</replaceable></term>
+ <listitem>
+ <para>
+ The tablespace where indexes will be rebuilt.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d776c661d7..4e5e3f7693 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -57,6 +57,7 @@
#include "commands/event_trigger.h"
#include "commands/progress.h"
#include "commands/tablecmds.h"
+#include "commands/tablespace.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -1394,9 +1395,13 @@ index_update_collation_versions(Oid relid, Oid coll)
* Create concurrently an index based on the definition of the one provided by
* caller. The index is inserted into catalogs and needs to be built later
* on. This is called during concurrent reindex processing.
+ *
+ * "tablespaceOid" is the new tablespace to use for this index. If
+ * InvalidOid, use the tablespace in-use instead.
*/
Oid
-index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char *newName)
+index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
+ Oid tablespaceOid, const char *newName)
{
Relation indexRelation;
IndexInfo *oldInfo,
@@ -1526,7 +1531,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char
newInfo,
indexColNames,
indexRelation->rd_rel->relam,
- indexRelation->rd_rel->reltablespace,
+ OidIsValid(tablespaceOid) ?
+ tablespaceOid : indexRelation->rd_rel->reltablespace,
indexRelation->rd_indcollation,
indclass->values,
indcoloptions->values,
@@ -3591,6 +3597,8 @@ IndexGetRelation(Oid indexId, bool missing_ok)
/*
* reindex_index - This routine is used to recreate a single index
+ *
+ * See comments of reindex_relation() for details about "tablespaceOid".
*/
void
reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
@@ -3599,10 +3607,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
Relation iRel,
heapRelation;
Oid heapId;
+ Oid oldTablespaceOid;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
PGRUsage ru0;
bool progress = ((options->flags & REINDEXOPT_REPORT_PROGRESS) != 0);
+ bool set_tablespace = OidIsValid(options->tablespaceOid);
pg_rusage_init(&ru0);
@@ -3654,6 +3664,35 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
get_namespace_name(RelationGetNamespace(iRel)),
RelationGetRelationName(iRel));
+ /*
+ * We don't support moving system relations into different tablespaces
+ * unless allow_system_table_mods=1.
+ */
+ if (set_tablespace &&
+ !allowSystemTableMods && IsSystemRelation(iRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(iRel))));
+
+ /*
+ * We cannot support moving mapped relations into different tablespaces.
+ * (In particular this eliminates all shared catalogs.)
+ */
+ if (set_tablespace && RelationIsMapped(iRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot change tablespace of mapped relation \"%s\"",
+ RelationGetRelationName(iRel))));
+
+ /* It's not a shared catalog, so refuse to move it to shared tablespace */
+ if (options->tablespaceOid == GLOBALTABLESPACE_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move non-shared relation to tablespace \"%s\"",
+ get_tablespace_name(options->tablespaceOid))));
+
+
/*
* Don't allow reindex on temp tables of other backends ... their local
* buffer manager is not going to cope.
@@ -3680,6 +3719,51 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
*/
CheckTableNotInUse(iRel, "REINDEX INDEX");
+ if (options->tablespaceOid == MyDatabaseTableSpace)
+ options->tablespaceOid = InvalidOid;
+
+ /*
+ * Set the new tablespace for the relation. Do that only in the
+ * case where the reindex caller wishes to enforce a new tablespace.
+ */
+ oldTablespaceOid = iRel->rd_rel->reltablespace;
+ if (set_tablespace &&
+ (options->tablespaceOid != oldTablespaceOid ||
+ (options->tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+ {
+ Relation pg_class;
+ Form_pg_class rd_rel;
+ HeapTuple tuple;
+
+ /* First get a modifiable copy of the relation's pg_class row */
+ pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+ tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", indexId);
+ rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+ /*
+ * Mark the relation as ready to be dropped at transaction commit,
+ * before making visible the new tablespace change so as this won't
+ * miss things.
+ */
+ RelationDropStorage(iRel);
+
+ /* Update the pg_class row */
+ rd_rel->reltablespace = options->tablespaceOid;
+ CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+ heap_freetuple(tuple);
+
+ table_close(pg_class, RowExclusiveLock);
+
+ RelationAssumeNewRelfilenode(iRel);
+
+ /* Make sure the reltablespace change is visible */
+ CommandCounterIncrement();
+ }
+
/*
* All predicate locks on the index are about to be made invalid. Promote
* them to relation locks on the heap.
@@ -3814,6 +3898,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* reindex_relation - This routine is used to recreate all indexes
* of a relation (and optionally its toast relation too, if any).
*
+ * "tablespaceOid" is the tablespace where the relation's indexes will be
+ * rebuilt, or InvalidOid to keep each index on its current tablespace.
+ *
* "flags" is a bitmask that can include any combination of these bits:
*
* REINDEX_REL_PROCESS_TOAST: if true, process the toast table too (if any).
@@ -3964,9 +4051,14 @@ reindex_relation(Oid relid, int flags, ReindexOptions *options)
/*
* Note that this should fail if the toast relation is missing, so
* reset REINDEXOPT_MISSING_OK.
+ *
+ * Even if a table's indexes were moved to a new tablespace, the index
+ * on its toast table is not normally moved.
*/
ReindexOptions newoptions = *options;
newoptions.flags &= ~(REINDEXOPT_MISSING_OK);
+ newoptions.tablespaceOid = allowSystemTableMods ?
+ options->tablespaceOid : InvalidOid;
result |= reindex_relation(toast_relid, flags, &newoptions);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index edd394ed4e..811ceb12b7 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2475,6 +2475,7 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
ListCell *lc;
bool concurrently = false;
bool verbose = false;
+ char *tablespace = NULL;
/* Parse option list */
foreach(lc, stmt->params)
@@ -2485,6 +2486,8 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
verbose = defGetBoolean(opt);
else if (strcmp(opt->defname, "concurrently") == 0)
concurrently = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "tablespace") == 0)
+ tablespace = defGetString(opt);
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -2501,6 +2504,9 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
(verbose ? REINDEXOPT_VERBOSE : 0) |
(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
+ options.tablespaceOid = tablespace != NULL ?
+ get_tablespace_oid(tablespace, false) : InvalidOid;
+
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
@@ -2728,7 +2734,10 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
MemoryContext old;
List *relids = NIL;
int num_keys;
+
bool concurrent_warning = false;
+ bool tablespace_warning = false;
+ bool mapped_warning = false;
AssertArg(objectName);
Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
@@ -2855,6 +2864,35 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue;
}
+ if (OidIsValid(options->tablespaceOid) &&
+ IsSystemClass(relid, classtuple))
+ {
+ if (!allowSystemTableMods)
+ {
+ /* Skip all system relations, if not allowSystemTableMods */
+ if (!tablespace_warning)
+ ereport(WARNING,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("cannot change tablespace of indexes on system relations, skipping all")));
+ tablespace_warning = true;
+ continue;
+ }
+ else if (!OidIsValid(classtuple->relfilenode))
+ {
+ /*
+ * Skip all mapped relations if TABLESPACE is specified.
+ * OidIsValid(relfilenode) checks that, similar to
+ * RelationIsMapped().
+ */
+ if (!mapped_warning)
+ ereport(WARNING,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot change tablespace of indexes on mapped relations, skipping all")));
+ mapped_warning = true;
+ continue;
+ }
+ }
+
/* Save the list of relation OIDs in private context */
old = MemoryContextSwitchTo(private_context);
@@ -2903,6 +2941,41 @@ reindex_error_callback(void *arg)
errinfo->relnamespace, errinfo->relname);
}
+/*
+ * This is mostly duplicating ATExecSetTableSpaceNoStorage,
+ * which should maybe be factored out to a library function.
+ */
+static void
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+ Relation pg_class;
+ HeapTuple tuple;
+ Form_pg_class rd_rel;
+ Oid oldTablespaceOid;
+
+ /* Get a modifiable copy of the relation's pg_class row */
+ pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+ tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", reloid);
+ rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+ /* No work if no change in tablespace. */
+ oldTablespaceOid = rd_rel->reltablespace;
+ if (tablespaceOid != oldTablespaceOid ||
+ (tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+ {
+ /* Update the pg_class row */
+ rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+ InvalidOid : tablespaceOid;
+ CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+ }
+
+ heap_freetuple(tuple);
+ table_close(pg_class, RowExclusiveLock);
+}
+
/*
* ReindexPartitions
*
@@ -2968,9 +3041,27 @@ ReindexPartitions(Oid relid, ReindexOptions *options, bool isTopLevel)
MemoryContext old_context;
/*
- * This discards partitioned tables, partitioned indexes and foreign
- * tables.
+ * Foreign tables and partitioned relations are not themselves
+ * reindexed - leaf partitions are processed directly. But any
+ * tablespace change is recorded in the catalog for partitioned
+ * relations.
*/
+ if (partkind == RELKIND_PARTITIONED_INDEX)
+ (void) set_rel_tablespace(partoid, options->tablespaceOid);
+ else if (partkind == RELKIND_PARTITIONED_TABLE)
+ {
+ Relation rel = table_open(partoid, ShareLock);
+ List *indexIds = RelationGetIndexList(rel);
+ ListCell *lc;
+
+ table_close(rel, NoLock);
+ foreach (lc, indexIds)
+ {
+ Oid indexid = lfirst_oid(lc);
+ (void) set_rel_tablespace(indexid, options->tablespaceOid);
+ }
+ }
+
if (!RELKIND_HAS_STORAGE(partkind))
continue;
@@ -3364,6 +3455,13 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options)
return false;
}
+ /* It's not a shared catalog, so refuse to move it to shared tablespace */
+ if (options->tablespaceOid == GLOBALTABLESPACE_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move non-shared relation to tablespace \"%s\"",
+ get_tablespace_name(options->tablespaceOid))));
+
Assert(heapRelationIds != NIL);
/*-----
@@ -3427,6 +3525,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options)
/* Create new index definition based on given index */
newIndexId = index_concurrently_create_copy(heapRel,
indexId,
+ options->tablespaceOid,
concurrentName);
/*
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3a43c09bf6..65ebf911f3 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3577,7 +3577,9 @@ psql_completion(const char *text, int start, int end)
* one word, so the above test is correct.
*/
if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
- COMPLETE_WITH("CONCURRENTLY", "VERBOSE");
+ COMPLETE_WITH("CONCURRENTLY", "TABLESPACE", "VERBOSE");
+ else if (TailMatches("TABLESPACE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
}
/* SECURITY LABEL */
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 89394b648e..b30887e02d 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -33,6 +33,7 @@ typedef enum
typedef struct ReindexOptions
{
bits32 flags; /* bitmask of REINDEXOPT_* */
+ Oid tablespaceOid; /* tablespace to rebuild index */
} ReindexOptions;
/* flag bits for ReindexOptions->flags */
@@ -92,6 +93,7 @@ extern Oid index_create(Relation heapRelation,
extern Oid index_concurrently_create_copy(Relation heapRelation,
Oid oldIndexId,
+ Oid tablespaceOid,
const char *newName);
extern void index_concurrently_build(Oid heapRelationId,
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index a5f61a35dc..5d8a22cffb 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -17,6 +17,48 @@ ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- f
ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail
ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok
+-- create table to test REINDEX with TABLESPACE change
+CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision);
+INSERT INTO regress_tblspace_test_tbl (num1, num2, num3)
+ SELECT round(random()*100), random(), random()*42
+ FROM generate_series(1, 20000) s(i);
+CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1);
+
+-- check that REINDEX with TABLESPACE change is transactional
+BEGIN;
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
+ROLLBACK;
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- first, let us reindex and move the entire database, after that return everything back
+REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning
+REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- check REINDEX with TABLESPACE change
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail
+REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail
+REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail
+
+-- check that all relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+
+-- move indexes back to pg_default tablespace
+REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok
+
+-- check that all relations moved back to pg_default
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
-- create a schema we can use
CREATE SCHEMA testschema;
@@ -96,6 +138,14 @@ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
\d testschema.part_a_idx
\d+ testschema.part_a_idx
+-- REINDEX partitioned indexes to new tablespace
+REINDEX (TABLESPACE pg_default) TABLE testschema.part;
+\d testschema.part
+\d testschema.part1
+REINDEX (CONCURRENTLY, TABLESPACE regress_tblspace) INDEX testschema.part_a_idx;
+\d testschema.part
+\d testschema.part1
+
-- partitioned rels cannot specify the default tablespace. These fail:
CREATE TABLE testschema.dflt (a int PRIMARY KEY) PARTITION BY LIST (a) TABLESPACE pg_default;
CREATE TABLE testschema.dflt (a int PRIMARY KEY USING INDEX TABLESPACE pg_default) PARTITION BY LIST (a);
@@ -279,6 +329,9 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
+DROP INDEX regress_tblspace_test_tbl_idx;
+DROP TABLE regress_tblspace_test_tbl;
+
DROP SCHEMA testschema CASCADE;
DROP ROLE regress_tablespace_user1;
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 162b591b31..1169f0318b 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -20,6 +20,65 @@ ERROR: unrecognized parameter "some_nonexistent_parameter"
ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail
ERROR: RESET must not include values for parameters
ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok
+-- create table to test REINDEX with TABLESPACE change
+CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision);
+INSERT INTO regress_tblspace_test_tbl (num1, num2, num3)
+ SELECT round(random()*100), random(), random()*42
+ FROM generate_series(1, 20000) s(i);
+CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1);
+-- check that REINDEX with TABLESPACE change is transactional
+BEGIN;
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
+ROLLBACK;
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
+-- first, let us reindex and move the entire database, after that return everything back
+REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning
+WARNING: cannot change tablespace of indexes on system relations, skipping all
+REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning
+WARNING: cannot change tablespace of indexes on system relations, skipping all
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
+-- check REINDEX with TABLESPACE change
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail
+ERROR: permission denied: "pg_authid_rolname_index" is a system catalog
+REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail
+ERROR: cannot reindex system catalogs concurrently
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail
+ERROR: cannot reindex system catalogs concurrently
+REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail
+ERROR: cannot move non-shared relation to tablespace "pg_global"
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail
+ERROR: permission denied: "pg_am_name_index" is a system catalog
+-- check that all relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+ relname
+-------------------------------
+ regress_tblspace_test_tbl_idx
+(1 row)
+
+-- move indexes back to pg_default tablespace
+REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok
+-- check that all relations moved back to pg_default
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
-- create a schema we can use
CREATE SCHEMA testschema;
-- try a table
@@ -199,6 +258,47 @@ Partitions: testschema.part1_a_idx,
testschema.part2_a_idx
Tablespace: "regress_tblspace"
+-- REINDEX partitioned indexes to new tablespace
+REINDEX (TABLESPACE pg_default) TABLE testschema.part;
+\d testschema.part
+ Partitioned table "testschema.part"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition key: LIST (a)
+Indexes:
+ "part_a_idx" btree (a)
+Number of partitions: 2 (Use \d+ to list them.)
+
+\d testschema.part1
+ Table "testschema.part1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: testschema.part FOR VALUES IN (1)
+Indexes:
+ "part1_a_idx" btree (a)
+
+REINDEX (CONCURRENTLY, TABLESPACE regress_tblspace) INDEX testschema.part_a_idx;
+\d testschema.part
+ Partitioned table "testschema.part"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition key: LIST (a)
+Indexes:
+ "part_a_idx" btree (a), tablespace "regress_tblspace"
+Number of partitions: 2 (Use \d+ to list them.)
+
+\d testschema.part1
+ Table "testschema.part1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: testschema.part FOR VALUES IN (1)
+Indexes:
+ "part1_a_idx" btree (a), tablespace "regress_tblspace"
+
-- partitioned rels cannot specify the default tablespace. These fail:
CREATE TABLE testschema.dflt (a int PRIMARY KEY) PARTITION BY LIST (a) TABLESPACE pg_default;
ERROR: cannot specify default tablespace for partitioned relations
@@ -736,6 +836,8 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
+DROP INDEX regress_tblspace_test_tbl_idx;
+DROP TABLE regress_tblspace_test_tbl;
DROP SCHEMA testschema CASCADE;
NOTICE: drop cascades to 6 other objects
DETAIL: drop cascades to table testschema.foo
--
2.17.0
view thread (154+ messages) latest in thread
Message-ID: <20201222211537.GO30237@telsasoft.com>
Permalink: ../20201222211537.GO30237@telsasoft.com/
Also on: postgresql.org/message-id/20201222211537.GO30237@telsasoft.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, michael@paquier.xyz, alvherre@alvh.no-ip.org, peter.eisentraut@enterprisedb.com, a.kondratov@postgrespro.ru, masahiko.sawada@2ndquadrant.com, steve@ssinger.info, robertmhaas@gmail.com, a.korotkov@postgrespro.ru, sawada.mshk@gmail.com, jltallon@adv-solutions.net
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
In-Reply-To: <20201222211537.GO30237@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox