On Sat, Dec 12, 2020 at 09:20:35AM +0100, Peter Eisentraut wrote: > On 2020-12-11 21:27, Alvaro Herrera wrote: > > By the way-- What did you think of the idea of explictly marking the > > types used for bitmasks using types bits32 and friends, instead of plain > > int, which is harder to spot? > > If we want to make it clearer, why not turn the thing into a struct, as in > the attached patch, and avoid the bit fiddling altogether.
I like this. It's a lot like what I wrote as [PATCH v31 1/5] ExecReindex and ReindexParams In my v31 patch, I moved ReindexOptions to a private structure in indexcmds.c, with an "int options" bitmask which is passed to reindex_index() et al. Your patch keeps/puts ReindexOptions index.h, so it also applies to reindex_index, which I think is good. So I've rebased this branch on your patch. Some thoughts: - what about removing the REINDEXOPT_* prefix ? - You created local vars with initialization like "={}". But I thought it's needed to include at least one struct member like "={false}", or else they're not guaranteed to be zerod ? - You passed the structure across function calls. The usual convention is to pass a pointer. I also changed the errcode and detail for this one. (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("incompatible TABLESPACE option"), errdetail("TABLESPACE can only be used with VACUUM FULL."))); -- Justin
>From 3d84ec564a1ad3df50967153d2acbd443a04b864 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Sat, 12 Dec 2020 09:17:55 +0100 Subject: [PATCH v34 1/8] Convert reindex options to struct --- src/backend/catalog/index.c | 19 ++++--- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 94 ++++++++++++++++---------------- src/backend/commands/tablecmds.c | 2 +- src/backend/tcop/utility.c | 4 +- src/include/catalog/index.h | 16 +++--- src/include/commands/defrem.h | 9 +-- 7 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 731610c701..06342fddf1 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.REINDEXOPT_REPORT_PROGRESS; 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.REINDEXOPT_MISSING_OK); /* if relation is missing, leave */ if (!OidIsValid(heapId)) return; - if ((options & REINDEXOPT_MISSING_OK) != 0) + if (options.REINDEXOPT_MISSING_OK) 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.REINDEXOPT_VERBOSE) 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.REINDEXOPT_MISSING_OK) 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.REINDEXOPT_MISSING_OK = false; + 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..b0aa3536d1 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1412,7 +1412,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, (ReindexOptions){}); /* 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..cd8eaa732d 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -89,9 +89,9 @@ 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 +100,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 */ }; @@ -2455,13 +2455,11 @@ ChooseIndexColumnNames(List *indexElems) * ReindexParseOptions * Parse list of REINDEX options, returning a bitmask of ReindexOption. */ -int +ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) { ListCell *lc; - int options = 0; - bool concurrently = false; - bool verbose = false; + ReindexOptions options = {}; /* Parse option list */ foreach(lc, stmt->params) @@ -2469,9 +2467,9 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) DefElem *opt = (DefElem *) lfirst(lc); if (strcmp(opt->defname, "verbose") == 0) - verbose = defGetBoolean(opt); + options.REINDEXOPT_VERBOSE = defGetBoolean(opt); else if (strcmp(opt->defname, "concurrently") == 0) - concurrently = defGetBoolean(opt); + options.REINDEXOPT_CONCURRENTLY = defGetBoolean(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -2480,10 +2478,6 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) parser_errposition(pstate, opt->location))); } - options = - (verbose ? REINDEXOPT_VERBOSE : 0) | - (concurrently ? REINDEXOPT_CONCURRENTLY : 0); - return options; } @@ -2492,7 +2486,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; @@ -2512,7 +2506,7 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel) state.options = options; state.locked_table_oid = InvalidOid; indOid = RangeVarGetRelidExtended(indexRelation, - (options & REINDEXOPT_CONCURRENTLY) != 0 ? + options.REINDEXOPT_CONCURRENTLY ? ShareUpdateExclusiveLock : AccessExclusiveLock, 0, RangeVarCallbackForReindexIndex, @@ -2527,12 +2521,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.REINDEXOPT_CONCURRENTLY && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else - reindex_index(indOid, false, persistence, - options | REINDEXOPT_REPORT_PROGRESS); + { + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_REPORT_PROGRESS = true; + reindex_index(indOid, false, persistence, newoptions); + } } /* @@ -2553,7 +2550,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.REINDEXOPT_CONCURRENTLY ? ShareUpdateExclusiveLock : ShareLock; /* @@ -2611,7 +2608,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 +2622,14 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel) * locks on our temporary table. */ heapOid = RangeVarGetRelidExtended(relation, - (options & REINDEXOPT_CONCURRENTLY) != 0 ? + options.REINDEXOPT_CONCURRENTLY ? 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.REINDEXOPT_CONCURRENTLY && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2644,10 +2641,12 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel) } else { + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_REPORT_PROGRESS = true; 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 +2666,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 +2685,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, objectKind == REINDEX_OBJECT_DATABASE); if (objectKind == REINDEX_OBJECT_SYSTEM && - (options & REINDEXOPT_CONCURRENTLY) != 0) + options.REINDEXOPT_CONCURRENTLY) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); @@ -2794,7 +2793,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.REINDEXOPT_CONCURRENTLY && IsCatalogRelationOid(relid)) { if (!concurrent_warning) @@ -2860,7 +2859,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 +2954,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 +2990,36 @@ ReindexMultipleInternal(List *relids, int options) Assert(relkind != RELKIND_PARTITIONED_INDEX && relkind != RELKIND_PARTITIONED_TABLE); - if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + if (options.REINDEXOPT_CONCURRENTLY && relpersistence != RELPERSISTENCE_TEMP) { - (void) ReindexRelationConcurrently(relid, - options | - REINDEXOPT_MISSING_OK); + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_MISSING_OK = true; + (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.REINDEXOPT_REPORT_PROGRESS = true; + newoptions.REINDEXOPT_MISSING_OK = true; + reindex_index(relid, false, relpersistence, newoptions); PopActiveSnapshot(); /* reindex_index() does the verbose output */ } else { bool result; + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_REPORT_PROGRESS = true; + newoptions.REINDEXOPT_MISSING_OK = true; 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.REINDEXOPT_VERBOSE) ereport(INFO, (errmsg("table \"%s.%s\" was reindexed", get_namespace_name(get_rel_namespace(relid)), @@ -3059,7 +3059,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 +3092,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) "ReindexConcurrent", ALLOCSET_SMALL_SIZES); - if (options & REINDEXOPT_VERBOSE) + if (options.REINDEXOPT_VERBOSE) { /* Save data needed by REINDEX VERBOSE in private context */ oldcontext = MemoryContextSwitchTo(private_context); @@ -3137,7 +3137,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.REINDEXOPT_MISSING_OK) { heapRelation = try_table_open(relationOid, ShareUpdateExclusiveLock); @@ -3233,7 +3233,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) case RELKIND_INDEX: { Oid heapId = IndexGetRelation(relationOid, - (options & REINDEXOPT_MISSING_OK) != 0); + options.REINDEXOPT_MISSING_OK); Relation heapRelation; /* if relation is missing, leave */ @@ -3262,7 +3262,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.REINDEXOPT_MISSING_OK) { heapRelation = try_table_open(heapId, ShareUpdateExclusiveLock); @@ -3754,7 +3754,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) StartTransactionCommand(); /* Log what we did */ - if (options & REINDEXOPT_VERBOSE) + if (options.REINDEXOPT_VERBOSE) { if (relkind == RELKIND_INDEX) ereport(INFO, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1fa9f19f08..9f218ac0e4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1891,7 +1891,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, (ReindexOptions){}); } pgstat_count_truncate(rel); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index a42ead7d69..e5a4e8f662 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -919,10 +919,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_ReindexStmt: { ReindexStmt *stmt = (ReindexStmt *) parsetree; - int options; + ReindexOptions options; options = ReindexParseOptions(pstate, stmt); - if ((options & REINDEXOPT_CONCURRENTLY) != 0) + if (options.REINDEXOPT_CONCURRENTLY) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index c041628049..81e3de4d22 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -30,13 +30,13 @@ 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; + bool REINDEXOPT_VERBOSE; /* print progress info */ + bool REINDEXOPT_REPORT_PROGRESS; /* report pgstat progress */ + bool REINDEXOPT_MISSING_OK; /* skip missing relations */ + bool REINDEXOPT_CONCURRENTLY; /* concurrent mode */ +} ReindexOptions; /* state info for validate_index bulkdelete callback */ typedef struct ValidateIndexState @@ -146,7 +146,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 +155,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/defrem.h b/src/include/commands/defrem.h index 1133ae1143..5b1a60d5fa 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,11 @@ 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 ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt); +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, - int options); + ReindexOptions options); extern char *makeObjectName(const char *name1, const char *name2, const char *label); extern char *ChooseRelationName(const char *name1, const char *name2, -- 2.17.0
>From c2606b731b399aa4700f4b7d0e1e0b79e6fedf59 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sat, 12 Dec 2020 11:42:19 -0600 Subject: [PATCH v34 2/8] fix --- src/backend/catalog/index.c | 23 ++++++----- src/backend/commands/cluster.c | 3 +- src/backend/commands/indexcmds.c | 68 ++++++++++++++++---------------- src/backend/commands/tablecmds.c | 4 +- src/backend/tcop/utility.c | 6 +-- src/include/catalog/index.h | 4 +- src/include/commands/defrem.h | 6 +-- 7 files changed, 58 insertions(+), 56 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 06342fddf1..da2f45b796 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, - ReindexOptions options) + ReindexOptions *options) { Relation iRel, heapRelation; @@ -3602,7 +3602,6 @@ 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; pg_rusage_init(&ru0); @@ -3611,12 +3610,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); + options->REINDEXOPT_MISSING_OK); /* if relation is missing, leave */ if (!OidIsValid(heapId)) return; - if (options.REINDEXOPT_MISSING_OK) + if (options->REINDEXOPT_MISSING_OK) heapRelation = try_table_open(heapId, ShareLock); else heapRelation = table_open(heapId, ShareLock); @@ -3625,7 +3624,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, if (!heapRelation) return; - if (progress) + if (options->REINDEXOPT_REPORT_PROGRESS) { pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId); @@ -3641,7 +3640,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, */ iRel = index_open(indexId, AccessExclusiveLock); - if (progress) + if (options->REINDEXOPT_REPORT_PROGRESS) pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID, iRel->rd_rel->relam); @@ -3792,14 +3791,14 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, } /* Log what we did */ - if (options.REINDEXOPT_VERBOSE) + if (options->REINDEXOPT_VERBOSE) ereport(INFO, (errmsg("index \"%s\" was reindexed", get_rel_name(indexId)), errdetail_internal("%s", pg_rusage_show(&ru0)))); - if (progress) + if (options->REINDEXOPT_REPORT_PROGRESS) pgstat_progress_end_command(); /* Close rels, but keep locks */ @@ -3846,7 +3845,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, * index rebuild. */ bool -reindex_relation(Oid relid, int flags, ReindexOptions options) +reindex_relation(Oid relid, int flags, ReindexOptions *options) { Relation rel; Oid toast_relid; @@ -3861,7 +3860,7 @@ reindex_relation(Oid relid, int flags, ReindexOptions options) * to prevent schema and data changes in it. The lock level used here * should match ReindexTable(). */ - if (options.REINDEXOPT_MISSING_OK) + if (options->REINDEXOPT_MISSING_OK) rel = try_table_open(relid, ShareLock); else rel = table_open(relid, ShareLock); @@ -3965,9 +3964,9 @@ reindex_relation(Oid relid, int flags, ReindexOptions options) * Note that this should fail if the toast relation is missing, so * reset REINDEXOPT_MISSING_OK. */ - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_MISSING_OK = false; - result |= reindex_relation(toast_relid, flags, newoptions); + result |= reindex_relation(toast_relid, flags, &newoptions); } return result; diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index b0aa3536d1..272723e050 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1353,6 +1353,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, char newrelpersistence) { ObjectAddress object; + ReindexOptions reindexopts = {false}; Oid mapped_tables[4]; int reindex_flags; int i; @@ -1412,7 +1413,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, (ReindexOptions){}); + reindex_relation(OIDOldHeap, reindex_flags, &reindexopts); /* 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 cd8eaa732d..d39840d493 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -89,9 +89,9 @@ 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, ReindexOptions options, bool isTopLevel); -static void ReindexMultipleInternal(List *relids, ReindexOptions options); -static bool ReindexRelationConcurrently(Oid relationOid, ReindexOptions 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); @@ -2459,7 +2459,7 @@ ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) { ListCell *lc; - ReindexOptions options = {}; + ReindexOptions options = {0}; /* Parse option list */ foreach(lc, stmt->params) @@ -2486,7 +2486,7 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) * Recreate a specific index. */ void -ReindexIndex(RangeVar *indexRelation, ReindexOptions options, bool isTopLevel) +ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel) { struct ReindexIndexCallbackState state; Oid indOid; @@ -2503,10 +2503,10 @@ ReindexIndex(RangeVar *indexRelation, ReindexOptions 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 ? + options->REINDEXOPT_CONCURRENTLY ? ShareUpdateExclusiveLock : AccessExclusiveLock, 0, RangeVarCallbackForReindexIndex, @@ -2521,14 +2521,14 @@ ReindexIndex(RangeVar *indexRelation, ReindexOptions options, bool isTopLevel) if (relkind == RELKIND_PARTITIONED_INDEX) ReindexPartitions(indOid, options, isTopLevel); - else if (options.REINDEXOPT_CONCURRENTLY && + else if (options->REINDEXOPT_CONCURRENTLY && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else { - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_REPORT_PROGRESS = true; - reindex_index(indOid, false, persistence, newoptions); + reindex_index(indOid, false, persistence, &newoptions); } } @@ -2608,7 +2608,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, * Recreate all indexes of a table (and of its toast table, if any) */ Oid -ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel) +ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel) { Oid heapOid; bool result; @@ -2622,14 +2622,14 @@ ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel) * locks on our temporary table. */ heapOid = RangeVarGetRelidExtended(relation, - options.REINDEXOPT_CONCURRENTLY ? + options->REINDEXOPT_CONCURRENTLY ? ShareUpdateExclusiveLock : ShareLock, 0, RangeVarCallbackOwnsTable, NULL); if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE) ReindexPartitions(heapOid, options, isTopLevel); - else if (options.REINDEXOPT_CONCURRENTLY && + else if (options->REINDEXOPT_CONCURRENTLY && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2641,12 +2641,12 @@ ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel) } else { - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_REPORT_PROGRESS = true; result = reindex_relation(heapOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, - newoptions); + &newoptions); if (!result) ereport(NOTICE, (errmsg("table \"%s\" has no indexes to reindex", @@ -2666,7 +2666,7 @@ ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel) */ void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - ReindexOptions options) + ReindexOptions *options) { Oid objectOid; Relation relationRelation; @@ -2685,7 +2685,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, objectKind == REINDEX_OBJECT_DATABASE); if (objectKind == REINDEX_OBJECT_SYSTEM && - options.REINDEXOPT_CONCURRENTLY) + options->REINDEXOPT_CONCURRENTLY) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); @@ -2793,7 +2793,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 && + if (options->REINDEXOPT_CONCURRENTLY && IsCatalogRelationOid(relid)) { if (!concurrent_warning) @@ -2859,7 +2859,7 @@ reindex_error_callback(void *arg) * by the caller. */ static void -ReindexPartitions(Oid relid, ReindexOptions options, bool isTopLevel) +ReindexPartitions(Oid relid, ReindexOptions *options, bool isTopLevel) { List *partitions = NIL; char relkind = get_rel_relkind(relid); @@ -2954,7 +2954,7 @@ ReindexPartitions(Oid relid, ReindexOptions options, bool isTopLevel) * and starts a new transaction when finished. */ static void -ReindexMultipleInternal(List *relids, ReindexOptions options) +ReindexMultipleInternal(List *relids, ReindexOptions *options) { ListCell *l; @@ -2990,36 +2990,36 @@ ReindexMultipleInternal(List *relids, ReindexOptions options) Assert(relkind != RELKIND_PARTITIONED_INDEX && relkind != RELKIND_PARTITIONED_TABLE); - if (options.REINDEXOPT_CONCURRENTLY && + if (options->REINDEXOPT_CONCURRENTLY && relpersistence != RELPERSISTENCE_TEMP) { - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_MISSING_OK = true; - (void) ReindexRelationConcurrently(relid, newoptions); + (void) ReindexRelationConcurrently(relid, &newoptions); /* ReindexRelationConcurrently() does the verbose output */ } else if (relkind == RELKIND_INDEX) { - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_REPORT_PROGRESS = true; newoptions.REINDEXOPT_MISSING_OK = true; - reindex_index(relid, false, relpersistence, newoptions); + reindex_index(relid, false, relpersistence, &newoptions); PopActiveSnapshot(); /* reindex_index() does the verbose output */ } else { bool result; - ReindexOptions newoptions = options; + ReindexOptions newoptions = *options; newoptions.REINDEXOPT_REPORT_PROGRESS = true; newoptions.REINDEXOPT_MISSING_OK = true; result = reindex_relation(relid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, - newoptions); + &newoptions); - if (result && options.REINDEXOPT_VERBOSE) + if (result && options->REINDEXOPT_VERBOSE) ereport(INFO, (errmsg("table \"%s.%s\" was reindexed", get_namespace_name(get_rel_namespace(relid)), @@ -3059,7 +3059,7 @@ ReindexMultipleInternal(List *relids, ReindexOptions options) * anyway, and a non-concurrent reindex is more efficient. */ static bool -ReindexRelationConcurrently(Oid relationOid, ReindexOptions options) +ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options) { List *heapRelationIds = NIL; List *indexIds = NIL; @@ -3092,7 +3092,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions options) "ReindexConcurrent", ALLOCSET_SMALL_SIZES); - if (options.REINDEXOPT_VERBOSE) + if (options->REINDEXOPT_VERBOSE) { /* Save data needed by REINDEX VERBOSE in private context */ oldcontext = MemoryContextSwitchTo(private_context); @@ -3137,7 +3137,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions options) errmsg("cannot reindex system catalogs concurrently"))); /* Open relation to get its indexes */ - if (options.REINDEXOPT_MISSING_OK) + if (options->REINDEXOPT_MISSING_OK) { heapRelation = try_table_open(relationOid, ShareUpdateExclusiveLock); @@ -3233,7 +3233,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions options) case RELKIND_INDEX: { Oid heapId = IndexGetRelation(relationOid, - options.REINDEXOPT_MISSING_OK); + options->REINDEXOPT_MISSING_OK); Relation heapRelation; /* if relation is missing, leave */ @@ -3262,7 +3262,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions 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) + if (options->REINDEXOPT_MISSING_OK) { heapRelation = try_table_open(heapId, ShareUpdateExclusiveLock); @@ -3754,7 +3754,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexOptions options) StartTransactionCommand(); /* Log what we did */ - if (options.REINDEXOPT_VERBOSE) + if (options->REINDEXOPT_VERBOSE) { if (relkind == RELKIND_INDEX) ereport(INFO, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9f218ac0e4..e0f62d3c77 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 reindexopts = {false}; /* Default options are all false */ /* * This effectively deletes all rows in the table, and may be done @@ -1891,7 +1892,8 @@ 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, (ReindexOptions){}); + reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, + &reindexopts); } pgstat_count_truncate(rel); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index e5a4e8f662..23612b7a90 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -929,10 +929,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, 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 81e3de4d22..3a8671f558 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -146,7 +146,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, ReindexOptions options); + char relpersistence, ReindexOptions *options); /* Flag bits for reindex_relation(): */ #define REINDEX_REL_PROCESS_TOAST 0x01 @@ -155,7 +155,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, ReindexOptions 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/defrem.h b/src/include/commands/defrem.h index 5b1a60d5fa..33df5d5780 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -36,10 +36,10 @@ extern ObjectAddress DefineIndex(Oid relationId, bool skip_build, bool quiet); extern ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt); -extern void ReindexIndex(RangeVar *indexRelation, ReindexOptions options, bool isTopLevel); -extern Oid ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel); +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); + ReindexOptions *options); extern char *makeObjectName(const char *name1, const char *name2, const char *label); extern char *ChooseRelationName(const char *name1, const char *name2, -- 2.17.0
>From 35bfb83f255cd22137ed0d92798e465a669bd888 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sat, 12 Dec 2020 11:42:14 -0600 Subject: [PATCH v34 3/8] ExecReindex and ReindexParams TODO: typedef --- src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++----- src/backend/tcop/utility.c | 40 +---------------------- src/include/commands/defrem.h | 6 +--- 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index d39840d493..ce0206a122 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -86,6 +86,11 @@ 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 Oid ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel); +static void ReindexMultipleTables(const char *objectName, + ReindexObjectType objectKind, ReindexOptions *options); static void RangeVarCallbackForReindexIndex(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); static void reindex_error_callback(void *args); @@ -2452,11 +2457,14 @@ ChooseIndexColumnNames(List *indexElems) } /* - * ReindexParseOptions - * Parse list of REINDEX options, returning a bitmask of ReindexOption. + * 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. */ -ReindexOptions -ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) +void +ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) { ListCell *lc; ReindexOptions options = {0}; @@ -2478,14 +2486,46 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt) parser_errposition(pstate, opt->location))); } - return options; + if (options.REINDEXOPT_CONCURRENTLY) + 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; + } } /* * ReindexIndex * Recreate a specific index. */ -void +static void ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel) { struct ReindexIndexCallbackState state; @@ -2607,7 +2647,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; @@ -2664,7 +2704,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 23612b7a90..3991a834b4 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -917,45 +917,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ReindexStmt: - { - ReindexStmt *stmt = (ReindexStmt *) parsetree; - ReindexOptions options; - - options = ReindexParseOptions(pstate, stmt); - if (options.REINDEXOPT_CONCURRENTLY) - 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, (ReindexStmt *)parsetree, isTopLevel); break; /* diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 33df5d5780..d4ea57e757 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -35,11 +35,7 @@ extern ObjectAddress DefineIndex(Oid relationId, bool check_not_in_use, bool skip_build, bool quiet); -extern ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt); -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
>From 9051b9beace44a98cbc9e02445eb12db3d73f7e5 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Mon, 23 Mar 2020 21:10:29 +0300 Subject: [PATCH v34 4/8] 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/backend/commands/tablecmds.c | 2 +- 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 +++++++++++++++++++++ 8 files changed, 378 insertions(+), 6 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 da2f45b796..aaac698eb0 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,9 +3607,11 @@ 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 set_tablespace = OidIsValid(options->tablespaceOid); pg_rusage_init(&ru0); @@ -3653,6 +3663,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. @@ -3679,6 +3718,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. @@ -3813,6 +3897,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). @@ -3963,9 +4050,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.REINDEXOPT_MISSING_OK = false; + 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 ce0206a122..849fecc861 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2468,6 +2468,7 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) { ListCell *lc; ReindexOptions options = {0}; + char *tablespace = NULL; /* Parse option list */ foreach(lc, stmt->params) @@ -2478,6 +2479,8 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) options.REINDEXOPT_VERBOSE = defGetBoolean(opt); else if (strcmp(opt->defname, "concurrently") == 0) options.REINDEXOPT_CONCURRENTLY = defGetBoolean(opt); + else if (strcmp(opt->defname, "tablespace") == 0) + tablespace = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -2490,6 +2493,9 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); + options.tablespaceOid = tablespace != NULL ? + get_tablespace_oid(tablespace, false) : InvalidOid; + switch (stmt->kind) { case REINDEX_OBJECT_INDEX: @@ -2717,7 +2723,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 || @@ -2844,6 +2853,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); @@ -2892,6 +2930,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 * @@ -2957,9 +3030,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; @@ -3354,6 +3445,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); /*----- @@ -3417,6 +3515,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/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e0f62d3c77..bb96e330d4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1854,7 +1854,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, { Oid heap_relid; Oid toast_relid; - ReindexOptions reindexopts = {false}; /* Default options are all false */ + ReindexOptions reindexopts = {false}; /* default options are all false */ /* * This effectively deletes all rows in the table, and may be done 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 3a8671f558..5db1d70064 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -36,6 +36,7 @@ typedef struct ReindexOptions bool REINDEXOPT_REPORT_PROGRESS; /* report pgstat progress */ bool REINDEXOPT_MISSING_OK; /* skip missing relations */ bool REINDEXOPT_CONCURRENTLY; /* concurrent mode */ + Oid tablespaceOid; /* tablespace to rebuild index */ } ReindexOptions; /* state info for validate_index bulkdelete callback */ @@ -89,6 +90,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
>From dea6b55a32571369820002b95dcabde3c2cdc317 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Wed, 23 Sep 2020 18:21:16 +0300 Subject: [PATCH v34 5/8] Refactor and reuse set_rel_tablespace() --- src/backend/catalog/index.c | 73 ++++++++++++++++++++------------ src/backend/commands/indexcmds.c | 35 --------------- src/include/catalog/index.h | 2 + 3 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index aaac698eb0..8911959336 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3607,7 +3607,6 @@ 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; @@ -3722,26 +3721,11 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, 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. + * Set the new tablespace for the relation if requested. */ - oldTablespaceOid = iRel->rd_rel->reltablespace; if (set_tablespace && - (options->tablespaceOid != oldTablespaceOid || - (options->tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))) + set_rel_tablespace(indexId, options->tablespaceOid)) { - 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 @@ -3749,14 +3733,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, */ 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 */ @@ -4064,6 +4040,51 @@ reindex_relation(Oid relid, int flags, ReindexOptions *options) return result; } +/* + * set_rel_tablespace - modify relation tablespace in the pg_class entry. + * + * 'reloid' is an Oid of relation to be modified. + * 'tablespaceOid' is an Oid of new tablespace. + * + * Catalog modification is done only if tablespaceOid is different from + * the currently set. Returned bool value is indicating whether any changes + * were made or not. + */ +bool +set_rel_tablespace(Oid reloid, Oid tablespaceOid) +{ + Relation pg_class; + HeapTuple tuple; + Form_pg_class rd_rel; + bool changed = false; + 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); + + changed = true; + } + + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + + return changed; +} /* ---------------------------------------------------------------- * System index reindexing support diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 849fecc861..2379d7d1a8 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2930,41 +2930,6 @@ 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 * diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 5db1d70064..9543ca9c61 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -150,6 +150,8 @@ extern Oid IndexGetRelation(Oid indexId, bool missing_ok); extern void reindex_index(Oid indexId, bool skip_constraint_checks, char relpersistence, ReindexOptions *options); +extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid); + /* Flag bits for reindex_relation(): */ #define REINDEX_REL_PROCESS_TOAST 0x01 #define REINDEX_REL_SUPPRESS_INDEX_USE 0x02 -- 2.17.0
>From 34e1e498853122c15f8ca5de95c9f34b06fd4b72 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Tue, 24 Mar 2020 18:16:06 +0300 Subject: [PATCH v34 6/8] Allow CLUSTER and VACUUM FULL to change tablespace --- doc/src/sgml/ref/cluster.sgml | 19 ++++--- doc/src/sgml/ref/vacuum.sgml | 20 +++++++ src/backend/commands/cluster.c | 67 +++++++++++++++++++++-- src/backend/commands/tablecmds.c | 5 +- src/backend/commands/vacuum.c | 55 ++++++++++++++++++- src/backend/parser/gram.y | 5 +- src/backend/postmaster/autovacuum.c | 1 + src/bin/psql/tab-complete.c | 9 ++- src/include/commands/cluster.h | 3 +- src/include/commands/vacuum.h | 2 + src/test/regress/input/tablespace.source | 23 +++++++- src/test/regress/output/tablespace.source | 37 ++++++++++++- 12 files changed, 221 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 5dd21a0189..6758ef97a6 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -28,6 +28,7 @@ CLUSTER [VERBOSE] <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] + TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> </synopsis> </refsynopsisdiv> @@ -105,6 +106,15 @@ CLUSTER [VERBOSE] </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the table will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>VERBOSE</literal></term> <listitem> @@ -115,15 +125,10 @@ CLUSTER [VERBOSE] </varlistentry> <varlistentry> - <term><replaceable class="parameter">boolean</replaceable></term> + <term><replaceable class="parameter">new_tablespace</replaceable></term> <listitem> <para> - Specifies whether the selected option should be turned on or off. - You can write <literal>TRUE</literal>, <literal>ON</literal>, or - <literal>1</literal> to enable the option, and <literal>FALSE</literal>, - <literal>OFF</literal>, or <literal>0</literal> to disable it. The - <replaceable class="parameter">boolean</replaceable> value can also - be omitted, in which case <literal>TRUE</literal> is assumed. + The tablespace where the table will be rebuilt. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 21ab57d880..5261a7c727 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -35,6 +35,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet INDEX_CLEANUP [ <replaceable class="parameter">boolean</replaceable> ] TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ] PARALLEL <replaceable class="parameter">integer</replaceable> + TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase> @@ -255,6 +256,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the relation will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">boolean</replaceable></term> <listitem> @@ -299,6 +309,16 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </para> </listitem> </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The tablespace where the relation will be rebuilt. + </para> + </listitem> + </varlistentry> + </variablelist> </refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 272723e050..39b32bb85f 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -33,11 +33,13 @@ #include "catalog/namespace.h" #include "catalog/objectaccess.h" #include "catalog/pg_am.h" +#include "catalog/pg_tablespace.h" #include "catalog/toasting.h" #include "commands/cluster.h" #include "commands/defrem.h" #include "commands/progress.h" #include "commands/tablecmds.h" +#include "commands/tablespace.h" #include "commands/vacuum.h" #include "miscadmin.h" #include "optimizer/optimizer.h" @@ -68,7 +70,8 @@ typedef struct } RelToCluster; -static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose); +static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, + Oid NewTableSpaceOid); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -105,6 +108,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) ListCell *lc; int options = 0; bool verbose = false; + /* Name and Oid of tablespace to use for clustered relation. */ + char *tablespaceName = NULL; + Oid tablespaceOid = InvalidOid; /* Parse option list */ foreach(lc, stmt->params) @@ -113,6 +119,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) if (strcmp(opt->defname, "verbose") == 0) verbose = defGetBoolean(opt); + else if (strcmp(opt->defname, "tablespace") == 0) + tablespaceName = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -123,6 +131,19 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) options = (verbose ? CLUOPT_VERBOSE : 0); + /* Select tablespace Oid to use. */ + if (tablespaceName) + { + tablespaceOid = get_tablespace_oid(tablespaceName, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespaceName))); + } + if (stmt->relation != NULL) { /* This is the single-relation case. */ @@ -192,7 +213,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) table_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, options); + cluster_rel(tableOid, indexOid, options, + tablespaceOid); } else { @@ -241,7 +263,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ cluster_rel(rvtc->tableOid, rvtc->indexOid, - options | CLUOPT_RECHECK); + options | CLUOPT_RECHECK, + tablespaceOid); PopActiveSnapshot(); CommitTransactionCommand(); } @@ -270,9 +293,12 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * If indexOid is InvalidOid, the table will be rewritten in physical order * instead of index order. This is the new implementation of VACUUM FULL, * and error messages should refer to the operation as VACUUM not CLUSTER. + * + * "tablespaceOid" is the tablespace where the relation will be rebuilt, or + * InvalidOid to use its current tablespace. */ void -cluster_rel(Oid tableOid, Oid indexOid, int options) +cluster_rel(Oid tableOid, Oid indexOid, int options, Oid tablespaceOid) { Relation OldHeap; bool verbose = ((options & CLUOPT_VERBOSE) != 0); @@ -372,6 +398,23 @@ cluster_rel(Oid tableOid, Oid indexOid, int options) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot cluster a shared catalog"))); + if (OidIsValid(tablespaceOid) && + !allowSystemTableMods && IsSystemRelation(OldHeap)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(OldHeap)))); + + /* + * We cannot support moving mapped relations into different tablespaces. + * (In particular this eliminates all shared catalogs.) + */ + if (OidIsValid(tablespaceOid) && RelationIsMapped(OldHeap)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(OldHeap)))); + /* * Don't process temp tables of other backends ... their local buffer * manager is not going to cope. @@ -422,7 +465,8 @@ cluster_rel(Oid tableOid, Oid indexOid, int options) TransferPredicateLocksToHeapRelation(OldHeap); /* rebuild_relation does all the dirty work */ - rebuild_relation(OldHeap, indexOid, verbose); + rebuild_relation(OldHeap, indexOid, verbose, + tablespaceOid); /* NB: rebuild_relation does table_close() on OldHeap */ @@ -571,7 +615,7 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * NB: this routine closes OldHeap at the right time; caller should not. */ static void -rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) +rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespaceOid) { Oid tableOid = RelationGetRelid(OldHeap); Oid tableSpace = OldHeap->rd_rel->reltablespace; @@ -582,6 +626,10 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) TransactionId frozenXid; MultiXactId cutoffMulti; + /* Use new tablespace if passed. */ + if (OidIsValid(NewTablespaceOid)) + tableSpace = NewTablespaceOid; + /* Mark the correct index as clustered */ if (OidIsValid(indexOid)) mark_index_clustered(OldHeap, indexOid, true); @@ -1026,6 +1074,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, */ Assert(!target_is_pg_class); + if (!allowSystemTableMods && IsSystemClass(r1, relform1) && + relform1->reltablespace != relform2->reltablespace) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + get_rel_name(r1)))); + swaptemp = relform1->relfilenode; relform1->relfilenode = relform2->relfilenode; relform2->relfilenode = swaptemp; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index bb96e330d4..3fb726db63 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13196,8 +13196,9 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) if (RelationIsMapped(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(rel)))); + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(rel)))); + /* Can't move a non-shared relation into pg_global */ if (newTableSpace == GLOBALTABLESPACE_OID) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 98270a1049..6861e948f2 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -31,13 +31,16 @@ #include "access/tableam.h" #include "access/transam.h" #include "access/xact.h" +#include "catalog/catalog.h" #include "catalog/namespace.h" #include "catalog/pg_database.h" #include "catalog/pg_inherits.h" #include "catalog/pg_namespace.h" +#include "catalog/pg_tablespace.h" #include "commands/cluster.h" #include "commands/defrem.h" #include "commands/vacuum.h" +#include "commands/tablespace.h" #include "miscadmin.h" #include "nodes/makefuncs.h" #include "pgstat.h" @@ -106,6 +109,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool disable_page_skipping = false; ListCell *lc; + /* Name and Oid of tablespace to use for relations after VACUUM FULL. */ + char *tablespacename = NULL; + Oid tablespaceOid = InvalidOid; + /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; params.truncate = VACOPT_TERNARY_DEFAULT; @@ -142,6 +149,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.index_cleanup = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "truncate") == 0) params.truncate = get_vacopt_ternary_value(opt); + else if (strcmp(opt->defname, "tablespace") == 0) + tablespacename = defGetString(opt); else if (strcmp(opt->defname, "parallel") == 0) { if (opt->arg == NULL) @@ -202,6 +211,27 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM FULL cannot be performed in parallel"))); + /* Get tablespace Oid to use. */ + if (tablespacename) + { + if ((params.options & VACOPT_FULL) == 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("incompatible TABLESPACE option"), + errdetail("TABLESPACE can only be used with VACUUM FULL."))); + + tablespaceOid = get_tablespace_oid(tablespacename, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespacename))); + + } + params.tablespace_oid = tablespaceOid; + /* * Make sure VACOPT_ANALYZE is specified if any column lists are present. */ @@ -1718,8 +1748,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) LOCKMODE lmode; Relation onerel; LockRelId onerelid; - Oid toast_relid; - Oid save_userid; + Oid toast_relid, + save_userid, + tablespaceOid = InvalidOid; int save_sec_context; int save_nestlevel; @@ -1857,6 +1888,23 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) return true; } + /* + * We don't support moving system relations into different tablespaces + * unless allow_system_table_mods=1. + */ + if (params->options & VACOPT_FULL && + OidIsValid(params->tablespace_oid) && + IsSystemRelation(onerel) && !allowSystemTableMods) + { + ereport(WARNING, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("skipping tablespace change of \"%s\"", + RelationGetRelationName(onerel)), + errdetail("Cannot move system relation, only VACUUM is performed."))); + } + else + tablespaceOid = params->tablespace_oid; + /* * Get a session-level lock too. This will protect our access to the * relation across multiple transactions, so that we can vacuum the @@ -1926,7 +1974,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) cluster_options |= 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, + tablespaceOid); } else table_relation_vacuum(onerel, params, vac_strategy); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 8f341ac006..2ef9485ab2 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10444,8 +10444,9 @@ cluster_index_specification: /***************************************************************************** * * QUERY: - * VACUUM - * ANALYZE + * VACUUM [FULL] [FREEZE] [VERBOSE] [ANALYZE] [ <table_and_columns> [, ...] ] + * VACUUM [(options)] [ <table_and_columns> [, ...] ] + * ANALYZE [VERBOSE] [ <table_and_columns> [, ...] ] * *****************************************************************************/ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 7e28944d2f..d2a443ebdb 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2933,6 +2933,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age; tab->at_params.is_wraparound = wraparound; tab->at_params.log_min_duration = log_min_duration; + tab->at_params.tablespace_oid = InvalidOid; tab->at_vacuum_cost_limit = vac_cost_limit; tab->at_vacuum_cost_delay = vac_cost_delay; tab->at_relname = NULL; diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 65ebf911f3..0908d7d4c7 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2312,7 +2312,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("VERBOSE"); + COMPLETE_WITH("TABLESPACE", "VERBOSE"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } /* COMMENT */ @@ -3806,9 +3808,12 @@ psql_completion(const char *text, int start, int end) if (ends_with(prev_wd, '(') || ends_with(prev_wd, ',')) COMPLETE_WITH("FULL", "FREEZE", "ANALYZE", "VERBOSE", "DISABLE_PAGE_SKIPPING", "SKIP_LOCKED", - "INDEX_CLEANUP", "TRUNCATE", "PARALLEL"); + "INDEX_CLEANUP", "TRUNCATE", "PARALLEL", + "TABLESPACE"); else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|INDEX_CLEANUP|TRUNCATE")) COMPLETE_WITH("ON", "OFF"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } else if (HeadMatches("VACUUM") && TailMatches("(")) /* "VACUUM (" should be caught above, so assume we want columns */ diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index 7cfb37c9b2..30ef24c41b 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -27,7 +27,8 @@ typedef enum ClusterOption } ClusterOption; 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, int options, + Oid tablespaceOid); 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/vacuum.h b/src/include/commands/vacuum.h index a4cd721400..4b5ac7145d 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -229,6 +229,8 @@ typedef struct VacuumParams * disabled. */ int nworkers; + Oid tablespace_oid; /* tablespace to use for relations + * rebuilt by VACUUM FULL */ } VacuumParams; /* GUC parameters */ diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index 5d8a22cffb..f4687f6bfb 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -17,7 +17,7 @@ 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 to test REINDEX, CLUSTER and VACUUM FULL 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 @@ -47,11 +47,32 @@ 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 indexes moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + +-- check CLUSTER with TABLESPACE change +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- 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; +-- check VACUUM with TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail + +-- check that all tables moved back to pg_default +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 diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 1169f0318b..19c9504c05 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -20,7 +20,7 @@ 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 to test REINDEX, CLUSTER and VACUUM FULL 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 @@ -61,9 +61,44 @@ 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 indexes 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) + +-- check CLUSTER with TABLESPACE change +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail +ERROR: cannot cluster a shared catalog +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" -- 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 + regress_tblspace_test_tbl_idx +(2 rows) + +-- check VACUUM with TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning +WARNING: skipping tablespace change of "pg_authid" +DETAIL: Cannot move system relation, only VACUUM is performed. +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail +ERROR: incompatible TABLESPACE option +DETAIL: TABLESPACE can only be used with VACUUM FULL. +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" +-- check that all tables moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') ORDER BY relname; relname ------------------------------- -- 2.17.0
>From 35010ec2bde928dae6d7f9625d15c37e88dc72d7 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Tue, 31 Mar 2020 20:35:41 -0500 Subject: [PATCH v34 7/8] Implement vacuum full/cluster (INDEX_TABLESPACE <tablespace>) --- doc/src/sgml/ref/cluster.sgml | 12 ++++- doc/src/sgml/ref/vacuum.sgml | 12 ++++- src/backend/commands/cluster.c | 64 ++++++++++++++--------- src/backend/commands/matview.c | 3 +- src/backend/commands/tablecmds.c | 2 +- src/backend/commands/vacuum.c | 46 +++++++--------- src/backend/postmaster/autovacuum.c | 1 + src/include/commands/cluster.h | 6 ++- src/include/commands/vacuum.h | 5 +- src/test/regress/input/tablespace.source | 13 +++++ src/test/regress/output/tablespace.source | 20 +++++++ 11 files changed, 123 insertions(+), 61 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 6758ef97a6..9c0f5add59 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -29,6 +29,7 @@ CLUSTER [VERBOSE] VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> + INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> </synopsis> </refsynopsisdiv> @@ -106,6 +107,15 @@ CLUSTER [VERBOSE] </listitem> </varlistentry> + <varlistentry> + <term><literal>INDEX_TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the table's indexes will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>TABLESPACE</literal></term> <listitem> @@ -128,7 +138,7 @@ CLUSTER [VERBOSE] <term><replaceable class="parameter">new_tablespace</replaceable></term> <listitem> <para> - The tablespace where the table will be rebuilt. + The tablespace where the table or its indexes will be rebuilt. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 5261a7c727..28cab119b6 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -36,6 +36,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ] PARALLEL <replaceable class="parameter">integer</replaceable> TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> + INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase> @@ -265,6 +266,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </listitem> </varlistentry> + <varlistentry> + <term><literal>INDEX_TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the relation's indexes will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">boolean</replaceable></term> <listitem> @@ -314,7 +324,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet <term><replaceable class="parameter">new_tablespace</replaceable></term> <listitem> <para> - The tablespace where the relation will be rebuilt. + The tablespace where the relation or its indexes will be rebuilt. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 39b32bb85f..ed57d3adfd 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -71,7 +71,7 @@ typedef struct static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, - Oid NewTableSpaceOid); + Oid NewTableSpaceOid, Oid NewIdxTableSpaceOid); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -108,9 +108,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) ListCell *lc; int options = 0; bool verbose = false; - /* Name and Oid of tablespace to use for clustered relation. */ - char *tablespaceName = NULL; - Oid tablespaceOid = InvalidOid; + /* Name and Oid of tablespaces to use for clustered relations. */ + char *tablespaceName = NULL, + *idxtablespaceName = NULL; + Oid tablespaceOid, + idxtablespaceOid; /* Parse option list */ foreach(lc, stmt->params) @@ -121,6 +123,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) verbose = defGetBoolean(opt); else if (strcmp(opt->defname, "tablespace") == 0) tablespaceName = defGetString(opt); + else if (strcmp(opt->defname, "index_tablespace") == 0) + idxtablespaceName = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -131,18 +135,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) options = (verbose ? CLUOPT_VERBOSE : 0); - /* Select tablespace Oid to use. */ - if (tablespaceName) - { - tablespaceOid = get_tablespace_oid(tablespaceName, false); - - /* Can't move a non-shared relation into pg_global */ - if (tablespaceOid == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move non-shared relation to tablespace \"%s\"", - tablespaceName))); - } + /* Get tablespaces to use. */ + tablespaceOid = tablespaceName ? + get_tablespace_oid(tablespaceName, false) : InvalidOid; + idxtablespaceOid = idxtablespaceName ? + get_tablespace_oid(idxtablespaceName, false) : InvalidOid; if (stmt->relation != NULL) { @@ -214,7 +211,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) /* Do the job. */ cluster_rel(tableOid, indexOid, options, - tablespaceOid); + tablespaceOid, idxtablespaceOid); } else { @@ -264,7 +261,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) /* Do the job. */ cluster_rel(rvtc->tableOid, rvtc->indexOid, options | CLUOPT_RECHECK, - tablespaceOid); + tablespaceOid, idxtablespaceOid); PopActiveSnapshot(); CommitTransactionCommand(); } @@ -294,11 +291,12 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * instead of index order. This is the new implementation of VACUUM FULL, * and error messages should refer to the operation as VACUUM not CLUSTER. * - * "tablespaceOid" is the tablespace where the relation will be rebuilt, or - * InvalidOid to use its current tablespace. + * "tablespaceOid" and "idxtablespaceOid" are the tablespaces where the relation + * and its indexes will be rebuilt, or InvalidOid to use their current + * tablespaces. */ void -cluster_rel(Oid tableOid, Oid indexOid, int options, Oid tablespaceOid) +cluster_rel(Oid tableOid, Oid indexOid, int options, Oid tablespaceOid, Oid idxtablespaceOid) { Relation OldHeap; bool verbose = ((options & CLUOPT_VERBOSE) != 0); @@ -466,7 +464,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options, Oid tablespaceOid) /* rebuild_relation does all the dirty work */ rebuild_relation(OldHeap, indexOid, verbose, - tablespaceOid); + tablespaceOid, idxtablespaceOid); /* NB: rebuild_relation does table_close() on OldHeap */ @@ -615,10 +613,11 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * NB: this routine closes OldHeap at the right time; caller should not. */ static void -rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespaceOid) +rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespaceOid, Oid NewIdxTablespaceOid) { Oid tableOid = RelationGetRelid(OldHeap); Oid tableSpace = OldHeap->rd_rel->reltablespace; + Oid idxtableSpace; Oid OIDNewHeap; char relpersistence; bool is_system_catalog; @@ -628,7 +627,20 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespace /* Use new tablespace if passed. */ if (OidIsValid(NewTablespaceOid)) + { tableSpace = NewTablespaceOid; + /* It's not a shared catalog, so refuse to move it to shared tablespace */ + if (tableSpace == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + get_tablespace_name(tableSpace)))); + } + + if (OidIsValid(NewIdxTablespaceOid)) + idxtableSpace = NewIdxTablespaceOid; + else + idxtableSpace = get_rel_tablespace(indexOid); /* Mark the correct index as clustered */ if (OidIsValid(indexOid)) @@ -657,7 +669,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespace finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog, swap_toast_by_content, false, true, frozenXid, cutoffMulti, - relpersistence); + relpersistence, idxtableSpace); } @@ -1405,10 +1417,10 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool is_internal, TransactionId frozenXid, MultiXactId cutoffMulti, - char newrelpersistence) + char newrelpersistence, Oid idxtableSpace) { ObjectAddress object; - ReindexOptions reindexopts = {false}; + ReindexOptions reindexopts = { .tablespaceOid = idxtableSpace }; Oid mapped_tables[4]; int reindex_flags; int i; diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index cfc63915f3..2630dcfbbc 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -855,7 +855,8 @@ static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence) { finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true, - RecentXmin, ReadNextMultiXactId(), relpersistence); + RecentXmin, ReadNextMultiXactId(), relpersistence, + InvalidOid); } /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3fb726db63..87563e0446 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5077,7 +5077,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, !OidIsValid(tab->newTableSpace), RecentXmin, ReadNextMultiXactId(), - persistence); + persistence, InvalidOid); } else { diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 6861e948f2..e4fa7df37c 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -109,9 +109,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool disable_page_skipping = false; ListCell *lc; - /* Name and Oid of tablespace to use for relations after VACUUM FULL. */ - char *tablespacename = NULL; - Oid tablespaceOid = InvalidOid; + /* Tablespace to use for relations after VACUUM FULL. */ + char *tablespacename = NULL, + *idxtablespacename = NULL; /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; @@ -151,6 +151,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.truncate = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "tablespace") == 0) tablespacename = defGetString(opt); + else if (strcmp(opt->defname, "index_tablespace") == 0) + idxtablespacename = defGetString(opt); else if (strcmp(opt->defname, "parallel") == 0) { if (opt->arg == NULL) @@ -211,26 +213,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM FULL cannot be performed in parallel"))); - /* Get tablespace Oid to use. */ - if (tablespacename) - { - if ((params.options & VACOPT_FULL) == 0) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("incompatible TABLESPACE option"), - errdetail("TABLESPACE can only be used with VACUUM FULL."))); - - tablespaceOid = get_tablespace_oid(tablespacename, false); - - /* Can't move a non-shared relation into pg_global */ - if (tablespaceOid == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move non-shared relation to tablespace \"%s\"", - tablespacename))); + if ((params.options & VACOPT_FULL) == 0 && + (tablespacename || idxtablespacename)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("incompatible TABLESPACE option"), + errdetail("TABLESPACE can only be used with VACUUM FULL."))); - } - params.tablespace_oid = tablespaceOid; + /* Get tablespace Oids to use. */ + params.tablespace_oid = tablespacename ? + get_tablespace_oid(tablespacename, false) : InvalidOid; + params.idxtablespace_oid = idxtablespacename ? + get_tablespace_oid(idxtablespacename, false) : InvalidOid; /* * Make sure VACOPT_ANALYZE is specified if any column lists are present. @@ -1749,8 +1743,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) Relation onerel; LockRelId onerelid; Oid toast_relid, - save_userid, - tablespaceOid = InvalidOid; + save_userid; int save_sec_context; int save_nestlevel; @@ -1896,14 +1889,13 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel) && !allowSystemTableMods) { + params->tablespace_oid = InvalidOid; ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("skipping tablespace change of \"%s\"", RelationGetRelationName(onerel)), errdetail("Cannot move system relation, only VACUUM is performed."))); } - else - tablespaceOid = params->tablespace_oid; /* * Get a session-level lock too. This will protect our access to the @@ -1975,7 +1967,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ cluster_rel(relid, InvalidOid, cluster_options, - tablespaceOid); + params->tablespace_oid, params->idxtablespace_oid); } else table_relation_vacuum(onerel, params, vac_strategy); diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index d2a443ebdb..0f566b997c 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2934,6 +2934,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.is_wraparound = wraparound; tab->at_params.log_min_duration = log_min_duration; tab->at_params.tablespace_oid = InvalidOid; + tab->at_params.idxtablespace_oid = InvalidOid; tab->at_vacuum_cost_limit = vac_cost_limit; tab->at_vacuum_cost_delay = vac_cost_delay; tab->at_relname = NULL; diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index 30ef24c41b..ab15689019 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -28,7 +28,8 @@ typedef enum ClusterOption extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); extern void cluster_rel(Oid tableOid, Oid indexOid, int options, - Oid tablespaceOid); + Oid tablespaceOid, + Oid indextablespaceOid); 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); @@ -42,6 +43,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool is_internal, TransactionId frozenXid, MultiXactId minMulti, - char newrelpersistence); + char newrelpersistence, + Oid idxtablespaceOid); #endif /* CLUSTER_H */ diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 4b5ac7145d..488d7540c7 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -229,8 +229,9 @@ typedef struct VacuumParams * disabled. */ int nworkers; - Oid tablespace_oid; /* tablespace to use for relations - * rebuilt by VACUUM FULL */ + /* tablespaces to use for relations rebuilt by VACUUM FULL */ + Oid tablespace_oid; + Oid idxtablespace_oid; } VacuumParams; /* GUC parameters */ diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index f4687f6bfb..3065e15e18 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -80,6 +80,19 @@ REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); +-- check CLUSTER with INDEX_TABLESPACE change to non-default location +CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +-- check relations moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; +-- check VACUUM with INDEX_TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +-- check 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; diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 19c9504c05..82a7f7a634 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -114,6 +114,26 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa --------- (0 rows) +-- check CLUSTER with INDEX_TABLESPACE change to non-default location +CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +-- check 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) + +-- check VACUUM with INDEX_TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +-- check 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 -- 2.17.0
>From c7f01df8d6c6baa42b69f14c2a50187667cf27d7 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 4 Dec 2020 15:02:59 -0600 Subject: [PATCH v34 8/8] Avoid enums for bitmasks.. ..since the utility enums is nullified --- contrib/bloom/blutils.c | 2 +- src/backend/access/common/reloptions.c | 12 ++--- src/backend/commands/explain.c | 2 +- src/backend/utils/sort/tuplesort.c | 5 +- src/include/access/reloptions.h | 43 ++++++++--------- src/include/access/relscan.h | 4 +- src/include/access/tableam.h | 47 +++++++++---------- src/include/catalog/index.h | 4 ++ src/include/catalog/namespace.h | 9 ++-- src/include/commands/cluster.h | 7 +-- src/include/commands/vacuum.h | 21 ++++----- src/include/executor/execdesc.h | 2 +- src/include/executor/instrument.h | 13 ++--- src/include/nodes/execnodes.h | 4 +- src/include/nodes/parsenodes.h | 24 +++++----- src/include/utils/tuplesort.h | 20 ++++---- .../modules/dummy_index_am/dummy_index_am.c | 2 +- src/tools/pgindent/typedefs.list | 2 - 18 files changed, 103 insertions(+), 120 deletions(-) diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c index 26b9927c3a..d8c575e5a8 100644 --- a/contrib/bloom/blutils.c +++ b/contrib/bloom/blutils.c @@ -35,7 +35,7 @@ PG_FUNCTION_INFO_V1(blhandler); /* Kind of relation options for bloom index */ -static relopt_kind bl_relopt_kind; +static int bl_relopt_kind; /* parse table for fillRelOptions */ static relopt_parse_elt bl_relopt_tab[INDEX_MAX_KEYS + 1]; diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 8ccc228a8c..c5dc7f5f52 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -652,10 +652,10 @@ initialize_reloptions(void) /* * add_reloption_kind - * Create a new relopt_kind value, to be used in custom reloptions by + * Return a value for a new kind of reloption, to be used in custom reloptions by * user-defined AMs. */ -relopt_kind +int add_reloption_kind(void) { /* don't hand out the last bit so that the enum's behavior is portable */ @@ -664,7 +664,7 @@ add_reloption_kind(void) (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("user-defined relation parameter types limit exceeded"))); last_assigned_kind <<= 1; - return (relopt_kind) last_assigned_kind; + return last_assigned_kind; } /* @@ -1475,7 +1475,7 @@ parseRelOptionsInternal(Datum options, bool validate, * be freed by the caller. */ static relopt_value * -parseRelOptions(Datum options, bool validate, relopt_kind kind, +parseRelOptions(Datum options, bool validate, int kind, int *numrelopts) { relopt_value *reloptions = NULL; @@ -1814,7 +1814,7 @@ fillRelOptions(void *rdopts, Size basesize, * Option parser for anything that uses StdRdOptions. */ bytea * -default_reloptions(Datum reloptions, bool validate, relopt_kind kind) +default_reloptions(Datum reloptions, bool validate, int kind) { static const relopt_parse_elt tab[] = { {"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)}, @@ -1885,7 +1885,7 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) */ void * build_reloptions(Datum reloptions, bool validate, - relopt_kind kind, + int kind, Size relopt_struct_size, const relopt_parse_elt *relopt_elems, int num_relopt_elems) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 43f9b01e83..5c6e9c98fd 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -2750,7 +2750,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, /* Generate a list of sort methods used across all groups. */ for (int bit = 0; bit < NUM_TUPLESORTMETHODS; bit++) { - TuplesortMethod sortMethod = (1 << bit); + int sortMethod = (1 << bit); if (groupInfo->sortMethods & sortMethod) { diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index d0cc04a878..285c94d34a 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -3397,10 +3397,11 @@ tuplesort_get_stats(Tuplesortstate *state, } /* - * Convert TuplesortMethod to a string. + * Convert sortMethod bitmask to a string. + * XXX: multiple bits can be defined */ const char * -tuplesort_method_name(TuplesortMethod m) +tuplesort_method_name(int m) { switch (m) { diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h index 5964438cb0..d7df48ef53 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -36,26 +36,23 @@ typedef enum relopt_type } relopt_type; /* kinds supported by reloptions */ -typedef enum relopt_kind -{ - RELOPT_KIND_LOCAL = 0, - RELOPT_KIND_HEAP = (1 << 0), - RELOPT_KIND_TOAST = (1 << 1), - RELOPT_KIND_BTREE = (1 << 2), - RELOPT_KIND_HASH = (1 << 3), - RELOPT_KIND_GIN = (1 << 4), - RELOPT_KIND_GIST = (1 << 5), - RELOPT_KIND_ATTRIBUTE = (1 << 6), - RELOPT_KIND_TABLESPACE = (1 << 7), - RELOPT_KIND_SPGIST = (1 << 8), - RELOPT_KIND_VIEW = (1 << 9), - RELOPT_KIND_BRIN = (1 << 10), - RELOPT_KIND_PARTITIONED = (1 << 11), - /* if you add a new kind, make sure you update "last_default" too */ - RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_PARTITIONED, - /* some compilers treat enums as signed ints, so we can't use 1 << 31 */ - RELOPT_KIND_MAX = (1 << 30) -} relopt_kind; +#define RELOPT_KIND_LOCAL 0 +#define RELOPT_KIND_HEAP (1 << 0) +#define RELOPT_KIND_TOAST (1 << 1) +#define RELOPT_KIND_BTREE (1 << 2) +#define RELOPT_KIND_HASH (1 << 3) +#define RELOPT_KIND_GIN (1 << 4) +#define RELOPT_KIND_GIST (1 << 5) +#define RELOPT_KIND_ATTRIBUTE (1 << 6) +#define RELOPT_KIND_TABLESPACE (1 << 7) +#define RELOPT_KIND_SPGIST (1 << 8) +#define RELOPT_KIND_VIEW (1 << 9) +#define RELOPT_KIND_BRIN (1 << 10) +#define RELOPT_KIND_PARTITIONED (1 << 11) +/* if you add a new kind, make sure you update "last_default" too */ +#define RELOPT_KIND_LAST_DEFAULT RELOPT_KIND_PARTITIONED +/* some compilers treat enums as signed ints, so we can't use 1 << 31 */ +#define RELOPT_KIND_MAX (1 << 30) /* reloption namespaces allowed for heaps -- currently only TOAST */ #define HEAP_RELOPT_NAMESPACES { "toast", NULL } @@ -179,7 +176,7 @@ typedef struct local_relopts ((optstruct)->member == 0 ? NULL : \ (char *)(optstruct) + (optstruct)->member) -extern relopt_kind add_reloption_kind(void); +extern int add_reloption_kind(void); extern void add_bool_reloption(bits32 kinds, const char *name, const char *desc, bool default_val, LOCKMODE lockmode); extern void add_int_reloption(bits32 kinds, const char *name, const char *desc, @@ -226,7 +223,7 @@ extern List *untransformRelOptions(Datum options); extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, amoptions_function amoptions); extern void *build_reloptions(Datum reloptions, bool validate, - relopt_kind kind, + int kind, Size relopt_struct_size, const relopt_parse_elt *relopt_elems, int num_relopt_elems); @@ -234,7 +231,7 @@ extern void *build_local_reloptions(local_relopts *relopts, Datum options, bool validate); extern bytea *default_reloptions(Datum reloptions, bool validate, - relopt_kind kind); + int kind); extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate); extern bytea *view_reloptions(Datum reloptions, bool validate); extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate); diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 5645976951..9a278e408f 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -37,8 +37,8 @@ typedef struct TableScanDescData struct ScanKeyData *rs_key; /* array of scan key descriptors */ /* - * Information about type and behaviour of the scan, a bitmask of members - * of the ScanOptions enum (see tableam.h). + * Information about type and behaviour of the scan, a bitmask of + * flags to scan_begin (see tableam.h). */ uint32 rs_flags; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 387eb34a61..eda47241eb 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -42,26 +42,25 @@ struct ValidateIndexState; /* * Bitmask values for the flags argument to the scan_begin callback. */ -typedef enum ScanOptions -{ - /* one of SO_TYPE_* may be specified */ - SO_TYPE_SEQSCAN = 1 << 0, - SO_TYPE_BITMAPSCAN = 1 << 1, - SO_TYPE_SAMPLESCAN = 1 << 2, - SO_TYPE_TIDSCAN = 1 << 3, - SO_TYPE_ANALYZE = 1 << 4, - - /* several of SO_ALLOW_* may be specified */ - /* allow or disallow use of access strategy */ - SO_ALLOW_STRAT = 1 << 5, - /* report location to syncscan logic? */ - SO_ALLOW_SYNC = 1 << 6, - /* verify visibility page-at-a-time? */ - SO_ALLOW_PAGEMODE = 1 << 7, - - /* unregister snapshot at scan end? */ - SO_TEMP_SNAPSHOT = 1 << 8 -} ScanOptions; + +/* one of SO_TYPE_* may be specified */ +#define SO_TYPE_SEQSCAN (1 << 0) +#define SO_TYPE_BITMAPSCAN (1 << 1) +#define SO_TYPE_SAMPLESCAN (1 << 2) +#define SO_TYPE_TIDSCAN (1 << 3) +#define SO_TYPE_ANALYZE (1 << 4) + +/* + * several of SO_ALLOW_* may be specified + */ +/* allow or disallow use of access strategy */ +#define SO_ALLOW_STRAT (1 << 5) +/* report location to syncscan logic? */ +#define SO_ALLOW_SYNC (1 << 6) +/* verify visibility page-at-a-time? */ +#define SO_ALLOW_PAGEMODE (1 << 7) +/* unregister snapshot at scan end? */ +#define SO_TEMP_SNAPSHOT (1 << 8) /* * Result codes for table_{update,delete,lock_tuple}, and for visibility @@ -192,11 +191,11 @@ typedef struct TableAmRoutine * parallelscan_initialize(), and has to be for the same relation. Will * only be set coming from table_beginscan_parallel(). * - * `flags` is a bitmask indicating the type of scan (ScanOptions's - * SO_TYPE_*, currently only one may be specified), options controlling - * the scan's behaviour (ScanOptions's SO_ALLOW_*, several may be + * `flags` is a bitmask indicating the type of scan + * (SO_TYPE_*, currently only one may be specified), options controlling + * the scan's behaviour (SO_ALLOW_*, several may be * specified, an AM may ignore unsupported ones) and whether the snapshot - * needs to be deallocated at scan_end (ScanOptions's SO_TEMP_SNAPSHOT). + * needs to be deallocated at scan_end (SO_TEMP_SNAPSHOT). */ TableScanDesc (*scan_begin) (Relation rel, Snapshot snapshot, diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 9543ca9c61..cd75249192 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -38,6 +38,10 @@ typedef struct ReindexOptions bool REINDEXOPT_CONCURRENTLY; /* concurrent mode */ Oid tablespaceOid; /* tablespace to rebuild index */ } ReindexOptions; +// #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ +// #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ +// #define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */ +// #define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */ /* state info for validate_index bulkdelete callback */ typedef struct ValidateIndexState diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 2456c08bf7..485293d77a 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -65,12 +65,9 @@ typedef struct OverrideSearchPath /* * Option flag bits for RangeVarGetRelidExtended(). */ -typedef enum RVROption -{ - RVR_MISSING_OK = 1 << 0, /* don't error if relation doesn't exist */ - RVR_NOWAIT = 1 << 1, /* error if relation cannot be locked */ - RVR_SKIP_LOCKED = 1 << 2 /* skip if relation cannot be locked */ -} RVROption; +#define RVR_MISSING_OK (1 << 0) /* don't error if relation doesn't exist */ +#define RVR_NOWAIT (1 << 1) /* error if relation cannot be locked */ +#define RVR_SKIP_LOCKED (1 << 2) /* skip if relation cannot be locked */ typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId, Oid oldRelId, void *callback_arg); diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index ab15689019..8f59af5863 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -20,11 +20,8 @@ /* options for CLUSTER */ -typedef enum ClusterOption -{ - CLUOPT_RECHECK = 1 << 0, /* recheck relation state */ - CLUOPT_VERBOSE = 1 << 1 /* print progress info */ -} ClusterOption; +#define CLUOPT_RECHECK (1 << 0) /* recheck relation state */ +#define CLUOPT_VERBOSE (1 << 1) /* print progress info */ extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); extern void cluster_rel(Oid tableOid, Oid indexOid, int options, diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 488d7540c7..0f9d4e2438 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -174,17 +174,14 @@ 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; +#define VACOPT_VACUUM (1 << 0) /* do VACUUM */ +#define VACOPT_ANALYZE (1 << 1) /* do ANALYZE */ +#define VACOPT_VERBOSE (1 << 2) /* print progress info */ +#define VACOPT_FREEZE (1 << 3) /* FREEZE option */ +#define VACOPT_FULL (1 << 4) /* FULL (non-concurrent) vacuum */ +#define VACOPT_SKIP_LOCKED (1 << 5) /* skip if cannot get lock */ +#define VACOPT_SKIPTOAST (1 << 6) /* don't process the TOAST table, if any */ +#define VACOPT_DISABLE_PAGE_SKIPPING (1 << 7) /* don't skip any pages */ /* * A ternary value used by vacuum parameters. @@ -207,7 +204,7 @@ typedef enum VacOptTernaryValue */ typedef struct VacuumParams { - int options; /* bitmask of VacuumOption */ + int options; /* bitmask of VACOPT_* options */ int freeze_min_age; /* min freeze age, -1 to use default */ int freeze_table_age; /* age at which to scan whole table */ int multixact_freeze_min_age; /* min multixact freeze age, -1 to diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h index b5cead3502..0087d192ca 100644 --- a/src/include/executor/execdesc.h +++ b/src/include/executor/execdesc.h @@ -41,7 +41,7 @@ typedef struct QueryDesc DestReceiver *dest; /* the destination for tuple output */ ParamListInfo params; /* param values being passed in */ QueryEnvironment *queryEnv; /* query environment passed in */ - int instrument_options; /* OR of InstrumentOption flags */ + int instrument_options; /* OR of flags to instrumentoption */ /* These fields are set by ExecutorStart */ TupleDesc tupDesc; /* descriptor for result tuples */ diff --git a/src/include/executor/instrument.h b/src/include/executor/instrument.h index 9dc3ecb07d..3148d88fea 100644 --- a/src/include/executor/instrument.h +++ b/src/include/executor/instrument.h @@ -40,14 +40,11 @@ typedef struct WalUsage } WalUsage; /* Flag bits included in InstrAlloc's instrument_options bitmask */ -typedef enum InstrumentOption -{ - INSTRUMENT_TIMER = 1 << 0, /* needs timer (and row counts) */ - INSTRUMENT_BUFFERS = 1 << 1, /* needs buffer usage */ - INSTRUMENT_ROWS = 1 << 2, /* needs row count */ - INSTRUMENT_WAL = 1 << 3, /* needs WAL usage */ - INSTRUMENT_ALL = PG_INT32_MAX -} InstrumentOption; +#define INSTRUMENT_TIMER (1 << 0) /* needs timer (and row counts) */ +#define INSTRUMENT_BUFFERS (1 << 1) /* needs buffer usage */ +#define INSTRUMENT_ROWS (1 << 2) /* needs row count */ +#define INSTRUMENT_WAL (1 << 3) /* needs WAL usage */ +#define INSTRUMENT_ALL PG_INT32_MAX typedef struct Instrumentation { diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 61ba4c3666..d065cdefd0 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -563,7 +563,7 @@ typedef struct EState uint64 es_processed; /* # of tuples processed */ int es_top_eflags; /* eflags passed to ExecutorStart */ - int es_instrument; /* OR of InstrumentOption flags */ + int es_instrument; /* OR of flags to instrument_options */ bool es_finished; /* true when ExecutorFinish is done */ List *es_exprcontexts; /* List of ExprContexts within EState */ @@ -2022,7 +2022,7 @@ typedef struct IncrementalSortGroupInfo int64 totalDiskSpaceUsed; int64 maxMemorySpaceUsed; int64 totalMemorySpaceUsed; - bits32 sortMethods; /* bitmask of TuplesortMethod */ + bits32 sortMethods; /* bitmask of sortMethod flags */ } IncrementalSortGroupInfo; typedef struct IncrementalSortInfo diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 48a79a7657..f223df8a0c 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -671,22 +671,20 @@ typedef struct TableLikeClause { NodeTag type; RangeVar *relation; - bits32 options; /* OR of TableLikeOption flags */ + bits32 options; /* OR of flags */ Oid relationOid; /* If table has been looked up, its OID */ } TableLikeClause; -typedef enum TableLikeOption -{ - CREATE_TABLE_LIKE_COMMENTS = 1 << 0, - CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 1, - CREATE_TABLE_LIKE_DEFAULTS = 1 << 2, - CREATE_TABLE_LIKE_GENERATED = 1 << 3, - CREATE_TABLE_LIKE_IDENTITY = 1 << 4, - CREATE_TABLE_LIKE_INDEXES = 1 << 5, - CREATE_TABLE_LIKE_STATISTICS = 1 << 6, - CREATE_TABLE_LIKE_STORAGE = 1 << 7, - CREATE_TABLE_LIKE_ALL = PG_INT32_MAX -} TableLikeOption; +/* bitmask of flags to TableLikeClause */ +#define CREATE_TABLE_LIKE_COMMENTS (1 << 0) +#define CREATE_TABLE_LIKE_CONSTRAINTS (1 << 1) +#define CREATE_TABLE_LIKE_DEFAULTS (1 << 2) +#define CREATE_TABLE_LIKE_GENERATED (1 << 3) +#define CREATE_TABLE_LIKE_IDENTITY (1 << 4) +#define CREATE_TABLE_LIKE_INDEXES (1 << 5) +#define CREATE_TABLE_LIKE_STATISTICS (1 << 6) +#define CREATE_TABLE_LIKE_STORAGE (1 << 7) +#define CREATE_TABLE_LIKE_ALL PG_INT32_MAX /* * IndexElem - index parameters (used in CREATE INDEX, and in ON CONFLICT) diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index c69b36e209..5da79fc0b6 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -62,21 +62,19 @@ typedef struct SortCoordinateData *SortCoordinate; * TuplesortInstrumentation can't contain any pointers because we * sometimes put it in shared memory. * - * The parallel-sort infrastructure relies on having a zero TuplesortMethod + * The parallel-sort infrastructure relies on having a zero sortMethod * to indicate that a worker never did anything, so we assign zero to * SORT_TYPE_STILL_IN_PROGRESS. The other values of this enum can be * OR'ed together to represent a situation where different workers used * different methods, so we need a separate bit for each one. Keep the * NUM_TUPLESORTMETHODS constant in sync with the number of bits! */ -typedef enum -{ - SORT_TYPE_STILL_IN_PROGRESS = 0, - SORT_TYPE_TOP_N_HEAPSORT = 1 << 0, - SORT_TYPE_QUICKSORT = 1 << 1, - SORT_TYPE_EXTERNAL_SORT = 1 << 2, - SORT_TYPE_EXTERNAL_MERGE = 1 << 3 -} TuplesortMethod; +/* sortMethod flags */ +#define SORT_TYPE_STILL_IN_PROGRESS 0 +#define SORT_TYPE_TOP_N_HEAPSORT (1 << 0) +#define SORT_TYPE_QUICKSORT (1 << 1) +#define SORT_TYPE_EXTERNAL_SORT (1 << 2) +#define SORT_TYPE_EXTERNAL_MERGE (1 << 3) #define NUM_TUPLESORTMETHODS 4 @@ -88,7 +86,7 @@ typedef enum typedef struct TuplesortInstrumentation { - TuplesortMethod sortMethod; /* sort algorithm used */ + int sortMethod; /* sort algorithm used */ TuplesortSpaceType spaceType; /* type of space spaceUsed represents */ int64 spaceUsed; /* space consumption, in kB */ } TuplesortInstrumentation; @@ -257,7 +255,7 @@ extern void tuplesort_reset(Tuplesortstate *state); extern void tuplesort_get_stats(Tuplesortstate *state, TuplesortInstrumentation *stats); -extern const char *tuplesort_method_name(TuplesortMethod m); +extern const char *tuplesort_method_name(int m); extern const char *tuplesort_space_type_name(TuplesortSpaceType t); extern int tuplesort_merge_order(int64 allowedMem); diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c b/src/test/modules/dummy_index_am/dummy_index_am.c index 8f4cdab1b3..3351639382 100644 --- a/src/test/modules/dummy_index_am/dummy_index_am.c +++ b/src/test/modules/dummy_index_am/dummy_index_am.c @@ -29,7 +29,7 @@ void _PG_init(void); relopt_parse_elt di_relopt_tab[6]; /* Kind of relation options for dummy index */ -relopt_kind di_relopt_kind; +int di_relopt_kind; typedef enum DummyAmEnum { diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a9dca717a6..229fa80675 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2583,7 +2583,6 @@ TupleQueueReader TupleTableSlot TupleTableSlotOps TuplesortInstrumentation -TuplesortMethod TuplesortSpaceType Tuplesortstate Tuplestorestate @@ -3309,7 +3308,6 @@ relopt_enum relopt_enum_elt_def relopt_gen relopt_int -relopt_kind relopt_parse_elt relopt_real relopt_string -- 2.17.0