Skip to content

Commit a3dc926

Browse files
michaelpqmichaelpq
committed
Refactor option handling of CLUSTER, REINDEX and VACUUM
This continues the work done in b5913f6. All the options of those commands are changed to use hex values rather than enums to reduce the risk of compatibility bugs when introducing new options. Each option set is moved into a new structure that can be extended with more non-boolean options (this was already the case of VACUUM). The code of REINDEX is restructured so as manual REINDEX commands go through a single routine from utility.c, like VACUUM, to ease the allocation handling of option parameters when a command needs to go through multiple transactions. This can be used as a base infrastructure for future patches related to those commands, including reindex filtering and tablespace support. Per discussion with people mentioned below, as well as Alvaro Herrera and Peter Eisentraut. Author: Michael Paquier, Justin Pryzby Reviewed-by: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/X8riynBLwxAD9uKk@paquier.xyz
1 parent 04eb75e commit a3dc926

File tree

11 files changed

+175
-149
lines changed

11 files changed

+175
-149
lines changed

‎src/backend/catalog/index.c

+12-10
Original file line numberDiff line numberDiff line change
@@ -3594,15 +3594,15 @@ IndexGetRelation(Oid indexId, bool missing_ok)
35943594
*/
35953595
void
35963596
reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
3597-
int options)
3597+
ReindexParams *params)
35983598
{
35993599
Relation iRel,
36003600
heapRelation;
36013601
Oid heapId;
36023602
IndexInfo *indexInfo;
36033603
volatile bool skipped_constraint = false;
36043604
PGRUsage ru0;
3605-
bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
3605+
bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
36063606

36073607
pg_rusage_init(&ru0);
36083608

@@ -3611,12 +3611,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
36113611
* we only need to be sure no schema or data changes are going on.
36123612
*/
36133613
heapId = IndexGetRelation(indexId,
3614-
(options & REINDEXOPT_MISSING_OK) != 0);
3614+
(params->options & REINDEXOPT_MISSING_OK) != 0);
36153615
/* if relation is missing, leave */
36163616
if (!OidIsValid(heapId))
36173617
return;
36183618

3619-
if ((options & REINDEXOPT_MISSING_OK) != 0)
3619+
if ((params->options & REINDEXOPT_MISSING_OK) != 0)
36203620
heapRelation = try_table_open(heapId, ShareLock);
36213621
else
36223622
heapRelation = table_open(heapId, ShareLock);
@@ -3792,7 +3792,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
37923792
}
37933793

37943794
/* Log what we did */
3795-
if (options & REINDEXOPT_VERBOSE)
3795+
if ((params->options & REINDEXOPT_VERBOSE) != 0)
37963796
ereport(INFO,
37973797
(errmsg("index \"%s\" was reindexed",
37983798
get_rel_name(indexId)),
@@ -3846,7 +3846,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
38463846
* index rebuild.
38473847
*/
38483848
bool
3849-
reindex_relation(Oid relid, int flags, int options)
3849+
reindex_relation(Oid relid, int flags, ReindexParams *params)
38503850
{
38513851
Relation rel;
38523852
Oid toast_relid;
@@ -3861,7 +3861,7 @@ reindex_relation(Oid relid, int flags, int options)
38613861
* to prevent schema and data changes in it. The lock level used here
38623862
* should match ReindexTable().
38633863
*/
3864-
if ((options & REINDEXOPT_MISSING_OK) != 0)
3864+
if ((params->options & REINDEXOPT_MISSING_OK) != 0)
38653865
rel = try_table_open(relid, ShareLock);
38663866
else
38673867
rel = table_open(relid, ShareLock);
@@ -3935,7 +3935,7 @@ reindex_relation(Oid relid, int flags, int options)
39353935
}
39363936

39373937
reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
3938-
persistence, options);
3938+
persistence, params);
39393939

39403940
CommandCounterIncrement();
39413941

@@ -3965,8 +3965,10 @@ reindex_relation(Oid relid, int flags, int options)
39653965
* Note that this should fail if the toast relation is missing, so
39663966
* reset REINDEXOPT_MISSING_OK.
39673967
*/
3968-
result |= reindex_relation(toast_relid, flags,
3969-
options & ~(REINDEXOPT_MISSING_OK));
3968+
ReindexParams newparams = *params;
3969+
3970+
newparams.options &= ~(REINDEXOPT_MISSING_OK);
3971+
result |= reindex_relation(toast_relid, flags, &newparams);
39703972
}
39713973

39723974
return result;

‎src/backend/commands/cluster.c

+11-8
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ void
103103
cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
104104
{
105105
ListCell *lc;
106-
int options = 0;
106+
ClusterParams params = {0};
107107
bool verbose = false;
108108

109109
/* Parse option list */
@@ -121,7 +121,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
121121
parser_errposition(pstate, opt->location)));
122122
}
123123

124-
options = (verbose ? CLUOPT_VERBOSE : 0);
124+
params.options = (verbose ? CLUOPT_VERBOSE : 0);
125125

126126
if (stmt->relation != NULL)
127127
{
@@ -192,7 +192,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
192192
table_close(rel, NoLock);
193193

194194
/* Do the job. */
195-
cluster_rel(tableOid, indexOid, options);
195+
cluster_rel(tableOid, indexOid, &params);
196196
}
197197
else
198198
{
@@ -234,14 +234,16 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
234234
foreach(rv, rvs)
235235
{
236236
RelToCluster *rvtc = (RelToCluster *) lfirst(rv);
237+
ClusterParams cluster_params = params;
237238

238239
/* Start a new transaction for each relation. */
239240
StartTransactionCommand();
240241
/* functions in indexes may want a snapshot set */
241242
PushActiveSnapshot(GetTransactionSnapshot());
242243
/* Do the job. */
244+
cluster_params.options |= CLUOPT_RECHECK;
243245
cluster_rel(rvtc->tableOid, rvtc->indexOid,
244-
options | CLUOPT_RECHECK);
246+
&cluster_params);
245247
PopActiveSnapshot();
246248
CommitTransactionCommand();
247249
}
@@ -272,11 +274,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
272274
* and error messages should refer to the operation as VACUUM not CLUSTER.
273275
*/
274276
void
275-
cluster_rel(Oid tableOid, Oid indexOid, int options)
277+
cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
276278
{
277279
Relation OldHeap;
278-
bool verbose = ((options & CLUOPT_VERBOSE) != 0);
279-
bool recheck = ((options & CLUOPT_RECHECK) != 0);
280+
bool verbose = ((params->options & CLUOPT_VERBOSE) != 0);
281+
bool recheck = ((params->options & CLUOPT_RECHECK) != 0);
280282

281283
/* Check for user-requested abort. */
282284
CHECK_FOR_INTERRUPTS();
@@ -1355,6 +1357,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
13551357
ObjectAddress object;
13561358
Oid mapped_tables[4];
13571359
int reindex_flags;
1360+
ReindexParams reindex_params = {0};
13581361
int i;
13591362

13601363
/* Report that we are now swapping relation files */
@@ -1412,7 +1415,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
14121415
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
14131416
PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
14141417

1415-
reindex_relation(OIDOldHeap, reindex_flags, 0);
1418+
reindex_relation(OIDOldHeap, reindex_flags, &reindex_params);
14161419

14171420
/* Report that we are now doing clean up */
14181421
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,

0 commit comments

Comments
 (0)