agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
Subject: [PATCH v17 7/8] pass idxtablespaceNAME to reduce error duplication
Date: Wed, 1 Apr 2020 19:00:25 -0500
Only reindex_index needs to look up the Oid or check for errors.
XXX: for cluster/vacuum, it might be more friendly to check before clustering
the table, rather than after clustering and re-indexing.
---
src/backend/catalog/index.c | 17 +++++++-------
src/backend/commands/cluster.c | 25 +++++++--------------
src/backend/commands/indexcmds.c | 35 +++++++++--------------------
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/vacuum.c | 15 ++++++-------
src/backend/postmaster/autovacuum.c | 2 +-
src/include/catalog/index.h | 4 ++--
src/include/commands/cluster.h | 4 ++--
src/include/commands/vacuum.h | 4 ++--
9 files changed, 41 insertions(+), 67 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d7db92219d..9e30e305bc 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3439,17 +3439,17 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* See comments of reindex_relation() for details about "tablespaceOid".
*/
void
-reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
+reindex_index(Oid indexId, const char *tablespace, bool skip_constraint_checks,
char persistence, int options)
{
Relation iRel,
heapRelation;
Oid heapId;
+ Oid tablespaceOid = tablespace ? get_tablespace_oid(tablespace, false) : InvalidOid;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
PGRUsage ru0;
bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
- bool set_tablespace = OidIsValid(tablespaceOid);
pg_rusage_init(&ru0);
@@ -3492,7 +3492,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
* We don't support moving system relations into different tablespaces,
* unless allow_system_table_mods=1.
*/
- if (set_tablespace &&
+ if (tablespace != NULL &&
!allowSystemTableMods && IsSystemRelation(iRel))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -3503,7 +3503,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
* We cannot support moving mapped relations into different tablespaces.
* (In particular this eliminates all shared catalogs.)
*/
- if (set_tablespace && RelationIsMapped(iRel))
+ if (tablespace != NULL && RelationIsMapped(iRel))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot change tablespace of mapped relation \"%s\"",
@@ -3516,7 +3516,6 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
errmsg("cannot move non-shared relation to tablespace \"%s\"",
get_tablespace_name(tablespaceOid))));
-
/*
* Don't allow reindex on temp tables of other backends ... their local
* buffer manager is not going to cope.
@@ -3550,7 +3549,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
* Set the new tablespace for the relation. Do that only in the
* case where the reindex caller wishes to enforce a new tablespace.
*/
- if (set_tablespace &&
+ if (tablespace != NULL &&
tablespaceOid != iRel->rd_rel->reltablespace)
{
Relation pg_class;
@@ -3758,7 +3757,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
* index rebuild.
*/
bool
-reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options)
+reindex_relation(Oid relid, const char *tablespace, int flags, int options)
{
Relation rel;
Oid toast_relid;
@@ -3849,7 +3848,7 @@ reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options)
continue;
}
- reindex_index(indexOid, tablespaceOid,
+ reindex_index(indexOid, tablespace,
!(flags & REINDEX_REL_CHECK_CONSTRAINTS),
persistence, options);
@@ -3883,7 +3882,7 @@ reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options)
* still hold the lock on the master table.
*/
if ((flags & REINDEX_REL_PROCESS_TOAST) && OidIsValid(toast_relid))
- result |= reindex_relation(toast_relid, tablespaceOid, flags, options);
+ result |= reindex_relation(toast_relid, tablespace, flags, options);
return result;
}
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 037b51e520..8601d36072 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -70,7 +70,7 @@ typedef struct
} RelToCluster;
-static void rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, Oid NewIdxTableSpaceOid, bool verbose);
+static void rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, const char *NewIdxTableSpace, bool verbose);
static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
@@ -109,7 +109,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
char *tablespaceName = NULL,
*idxtablespaceName = NULL;
Oid tablespaceOid;
- Oid idxtablespaceOid;
/* Parse list of generic parameter not handled by the parser */
foreach(lc, stmt->params)
@@ -134,8 +133,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
/* Get tablespaces to use. */
tablespaceOid = tablespaceName ?
get_tablespace_oid(tablespaceName, false) : InvalidOid;
- idxtablespaceOid = idxtablespaceName ?
- get_tablespace_oid(idxtablespaceName, false) : InvalidOid;
if (stmt->relation != NULL)
{
@@ -218,7 +215,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
table_close(rel, NoLock);
/* Do the job. */
- cluster_rel(tableOid, indexOid, tablespaceOid, idxtablespaceOid, stmt->options);
+ cluster_rel(tableOid, indexOid, tablespaceOid, idxtablespaceName, stmt->options);
}
else
{
@@ -267,7 +264,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
PushActiveSnapshot(GetTransactionSnapshot());
/* Do the job. */
cluster_rel(rvtc->tableOid, rvtc->indexOid, tablespaceOid,
- idxtablespaceOid, stmt->options | CLUOPT_RECHECK);
+ idxtablespaceName, stmt->options | CLUOPT_RECHECK);
PopActiveSnapshot();
CommitTransactionCommand();
}
@@ -301,7 +298,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
* InvalidOid, use the tablespace in-use instead.
*/
void
-cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, Oid idxtablespaceOid, int options)
+cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, const char *idxtablespace, int options)
{
Relation OldHeap;
bool verbose = ((options & CLUOPT_VERBOSE) != 0);
@@ -481,7 +478,7 @@ cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, Oid idxtablespaceOid,
TransferPredicateLocksToHeapRelation(OldHeap);
/* rebuild_relation does all the dirty work */
- rebuild_relation(OldHeap, indexOid, tablespaceOid, idxtablespaceOid, verbose);
+ rebuild_relation(OldHeap, indexOid, tablespaceOid, idxtablespace, verbose);
/* NB: rebuild_relation does table_close() on OldHeap */
@@ -640,11 +637,10 @@ 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, Oid NewTablespaceOid, Oid NewIdxTablespaceOid, bool verbose)
+rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, const char *NewIdxTablespace, bool verbose)
{
Oid tableOid = RelationGetRelid(OldHeap);
Oid tableSpace = OldHeap->rd_rel->reltablespace;
- Oid idxtableSpace;
Oid OIDNewHeap;
char relpersistence;
bool is_system_catalog;
@@ -664,11 +660,6 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, Oid NewId
get_tablespace_name(tableSpace))));
}
- if (OidIsValid(NewIdxTablespaceOid))
- idxtableSpace = NewIdxTablespaceOid;
- else
- idxtableSpace = get_rel_tablespace(indexOid);
-
/* Mark the correct index as clustered */
if (OidIsValid(indexOid))
mark_index_clustered(OldHeap, indexOid, true);
@@ -696,7 +687,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, Oid NewId
finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
swap_toast_by_content, false, true,
frozenXid, cutoffMulti,
- relpersistence, idxtableSpace);
+ relpersistence, NewIdxTablespace);
}
@@ -1425,7 +1416,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
bool is_internal,
TransactionId frozenXid,
MultiXactId cutoffMulti,
- char newrelpersistence, Oid idxtableSpace)
+ char newrelpersistence, const char *idxtableSpace)
{
ObjectAddress object;
Oid mapped_tables[4];
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7e4e54902e..3b07129f30 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -87,7 +87,7 @@ static char *ChooseIndexNameAddition(List *colnames);
static List *ChooseIndexColumnNames(List *indexElems);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
-static bool ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options);
+static bool ReindexRelationConcurrently(Oid relationOid, const char *tablespace, int options);
static void ReindexPartitionedIndex(Relation parentIdx);
static void update_relispartition(Oid relationId, bool newval);
static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
@@ -2423,7 +2423,6 @@ ReindexIndex(ReindexStmt *stmt)
RangeVar *indexRelation = stmt->relation;
struct ReindexIndexCallbackState state;
Oid indOid;
- Oid tablespaceOid = InvalidOid;
Relation irel;
char persistence;
@@ -2459,16 +2458,12 @@ ReindexIndex(ReindexStmt *stmt)
persistence = irel->rd_rel->relpersistence;
- /* Define new tablespaceOid if requested */
- if (stmt->tablespacename)
- tablespaceOid = get_tablespace_oid(stmt->tablespacename, false);
-
index_close(irel, NoLock);
if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
- ReindexRelationConcurrently(indOid, tablespaceOid, stmt->options);
+ ReindexRelationConcurrently(indOid, stmt->tablespacename, stmt->options);
else
- reindex_index(indOid, tablespaceOid, false, persistence,
+ reindex_index(indOid, stmt->tablespacename, false, persistence,
stmt->options | REINDEXOPT_REPORT_PROGRESS);
}
@@ -2552,7 +2547,6 @@ ReindexTable(ReindexStmt *stmt)
RangeVar *relation = stmt->relation;
Oid heapOid;
bool result;
- Oid tablespaceOid = InvalidOid;
/*
* The lock level used here should match reindex_relation().
@@ -2567,13 +2561,9 @@ ReindexTable(ReindexStmt *stmt)
0,
RangeVarCallbackOwnsTable, NULL);
- /* Define new tablespaceOid if requested */
- if (stmt->tablespacename)
- tablespaceOid = get_tablespace_oid(stmt->tablespacename, false);
-
if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
- result = ReindexRelationConcurrently(heapOid, tablespaceOid, stmt->options);
+ result = ReindexRelationConcurrently(heapOid, stmt->tablespacename, stmt->options);
if (!result)
ereport(NOTICE,
@@ -2583,7 +2573,7 @@ ReindexTable(ReindexStmt *stmt)
else
{
result = reindex_relation(heapOid,
- tablespaceOid,
+ stmt->tablespacename,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
stmt->options | REINDEXOPT_REPORT_PROGRESS);
@@ -2610,7 +2600,6 @@ ReindexMultipleTables(ReindexStmt *stmt)
const char *objectName = stmt->name;
ReindexObjectType objectKind = stmt->kind;
Oid objectOid;
- Oid tablespaceOid = InvalidOid;
Relation relationRelation;
TableScanDesc scan;
ScanKeyData scan_keys[1];
@@ -2660,10 +2649,6 @@ ReindexMultipleTables(ReindexStmt *stmt)
objectName);
}
- /* Define new tablespaceOid if requested */
- if (stmt->tablespacename)
- tablespaceOid = get_tablespace_oid(stmt->tablespacename, false);
-
/*
* Create a memory context that will survive forced transaction commits we
* do below. Since it is a child of PortalContext, it will go away
@@ -2754,7 +2739,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
continue;
}
- if (OidIsValid(tablespaceOid) &&
+ if (stmt->tablespacename && // OidIsValid(tablespaceOid) &&
IsSystemClass(relid, classtuple))
{
if (!allowSystemTableMods)
@@ -2816,7 +2801,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{
- (void) ReindexRelationConcurrently(relid, tablespaceOid, stmt->options);
+ (void) ReindexRelationConcurrently(relid, stmt->tablespacename, stmt->options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
@@ -2824,7 +2809,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
bool result;
result = reindex_relation(relid,
- tablespaceOid,
+ stmt->tablespacename,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
stmt->options | REINDEXOPT_REPORT_PROGRESS);
@@ -2874,7 +2859,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
* anyway, and a non-concurrent reindex is more efficient.
*/
static bool
-ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
+ReindexRelationConcurrently(Oid relationOid, const char *tablespace, int options)
{
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3148,7 +3133,7 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
/* Create new index definition based on given index */
newIndexId = index_concurrently_create_copy(heapRel,
indexId,
- tablespaceOid,
+ tablespace ? get_tablespace_oid(tablespace, false) : InvalidOid,
concurrentName);
/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 642f515655..c1911af3f2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1871,7 +1871,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
/*
* Reconstruct the indexes to match, and we're done.
*/
- reindex_relation(heap_relid, InvalidOid, REINDEX_REL_PROCESS_TOAST, 0);
+ reindex_relation(heap_relid, NULL, REINDEX_REL_PROCESS_TOAST, 0);
}
pgstat_count_truncate(rel);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index b79aa3d30f..c8a307b1a9 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -111,8 +111,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
ListCell *lc;
/* Tablespace to use for relations after VACUUM FULL. */
- char *tablespacename = NULL,
- *idxtablespacename = NULL;
+ char *tablespacename = NULL;
/* Set default value */
params.index_cleanup = VACOPT_TERNARY_DEFAULT;
@@ -121,6 +120,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* By default parallel vacuum is enabled */
params.nworkers = 0;
+ params.idxtablespace = NULL;
+
/* Parse options list */
foreach(lc, vacstmt->options)
{
@@ -153,7 +154,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
else if (strcmp(opt->defname, "tablespace") == 0)
tablespacename = defGetString(opt);
else if (strcmp(opt->defname, "index_tablespace") == 0)
- idxtablespacename = defGetString(opt);
+ params.idxtablespace = defGetString(opt);
else if (strcmp(opt->defname, "parallel") == 0)
{
parallel_option = true;
@@ -216,17 +217,15 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
errmsg("cannot specify both FULL and PARALLEL options")));
if ((params.options & VACOPT_FULL) == 0 &&
- (tablespacename || idxtablespacename))
+ (tablespacename || params.idxtablespace))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("incompatible TABLESPACE option"),
errdetail("You can only use TABLESPACE with VACUUM FULL.")));
- /* Get tablespace Oids to use. */
+ /* Get tablespaces 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.
@@ -1917,7 +1916,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
cluster_rel(relid, InvalidOid, params->tablespace_oid,
- params->idxtablespace_oid, cluster_options);
+ params->idxtablespace, cluster_options);
}
else
table_relation_vacuum(onerel, params, vac_strategy);
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7de5797f9c..8f191c9ead 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2898,7 +2898,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_params.idxtablespace = NULL;
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/catalog/index.h b/src/include/catalog/index.h
index f38e978b45..35301cf47b 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -132,7 +132,7 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
-extern void reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
+extern void reindex_index(Oid indexId, const char *tablespace, bool skip_constraint_checks,
char relpersistence, int options);
/* Flag bits for reindex_relation(): */
@@ -142,7 +142,7 @@ extern void reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_c
#define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08
#define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
-extern bool reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options);
+extern bool reindex_relation(Oid relid, const char *tablespace, int flags, int options);
extern bool ReindexIsProcessingHeap(Oid heapOid);
extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index 515e810505..80f9120138 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -20,7 +20,7 @@
extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel);
-extern void cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, Oid indextablespaceOid, int options);
+extern void cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, const char *indextablespace, int options);
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
bool recheck, LOCKMODE lockmode);
extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
@@ -35,6 +35,6 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
TransactionId frozenXid,
MultiXactId minMulti,
char newrelpersistence,
- Oid idxtablespaceOid);
+ const char *idxtablespace);
#endif /* CLUSTER_H */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index d6f8e5de23..edbb123681 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -230,9 +230,9 @@ typedef struct VacuumParams
*/
int nworkers;
- /* tablespace Oids to use for relations rebuilt by VACUUM FULL */
+ /* tablespaces to use for relations rebuilt by VACUUM FULL */
Oid tablespace_oid;
- Oid idxtablespace_oid;
+ char *idxtablespace;
} VacuumParams;
/* GUC parameters */
--
2.17.0
--byLs0wutDcxFdwtm
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v17-0008-Also-pass-the-table-s-tablespace-as-char.patch"
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v17 7/8] pass idxtablespaceNAME to reduce error duplication
In-Reply-To: <no-message-id-194660@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox