From 9ebe0572ceab69c57811746ead2d3418daea8673 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 24 Jul 2018 11:37:32 +0900 Subject: [PATCH] Refactor cluster_rel() to handle more options This extends cluster_rel() in such a way that more options can be added in the future, which will reduce the amount of chunk code for an upcoming SKIP_LOCKED aimed for VACUUM. As VACUUM FULL is a different flavor of CLUSTER, we want to make that extensible to ease integration. This only reworks the API and its callers, without providing anything user-facing. Two options are present now: verbose mode and relation recheck when doing the cluster command work across multiple transactions. This could be used as well as a base to extend the grammar of CLUSTER later on. Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/20180723031058.GE2854@paquier.xyz --- src/backend/commands/cluster.c | 9 ++++++--- src/backend/commands/vacuum.c | 8 ++++++-- src/backend/nodes/copyfuncs.c | 2 +- src/backend/nodes/equalfuncs.c | 2 +- src/backend/parser/gram.y | 12 +++++++++--- src/include/commands/cluster.h | 3 +-- src/include/nodes/parsenodes.h | 8 +++++++- src/tools/pgindent/typedefs.list | 1 + 8 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 0112a87224..68be470977 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -186,7 +186,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) heap_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, false, stmt->verbose); + cluster_rel(tableOid, indexOid, stmt->options); } else { @@ -234,7 +234,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel) /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ - cluster_rel(rvtc->tableOid, rvtc->indexOid, true, stmt->verbose); + cluster_rel(rvtc->tableOid, rvtc->indexOid, + stmt->options | CLUOPT_RECHECK); PopActiveSnapshot(); CommitTransactionCommand(); } @@ -265,9 +266,11 @@ cluster(ClusterStmt *stmt, bool isTopLevel) * and error messages should refer to the operation as VACUUM not CLUSTER. */ void -cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose) +cluster_rel(Oid tableOid, Oid indexOid, int options) { Relation OldHeap; + bool verbose = ((options & CLUOPT_VERBOSE) != 0); + bool recheck = ((options & CLUOPT_RECHECK) != 0); /* Check for user-requested abort. */ CHECK_FOR_INTERRUPTS(); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index bd0f04c7e2..5736f12b8f 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1551,13 +1551,17 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params) */ if (options & VACOPT_FULL) { + int options = 0; + /* close relation before vacuuming, but hold lock until commit */ relation_close(onerel, NoLock); onerel = NULL; + if ((options & VACOPT_VERBOSE) != 0) + options |= CLUOPT_VERBOSE; + /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ - cluster_rel(relid, InvalidOid, false, - (options & VACOPT_VERBOSE) != 0); + cluster_rel(relid, InvalidOid, options); } else lazy_vacuum_rel(onerel, options, params, vac_strategy); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 96836ef19c..17b650b8cb 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3284,7 +3284,7 @@ _copyClusterStmt(const ClusterStmt *from) COPY_NODE_FIELD(relation); COPY_STRING_FIELD(indexname); - COPY_SCALAR_FIELD(verbose); + COPY_SCALAR_FIELD(options); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 6a971d0141..378f2facb8 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1206,7 +1206,7 @@ _equalClusterStmt(const ClusterStmt *a, const ClusterStmt *b) { COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(indexname); - COMPARE_SCALAR_FIELD(verbose); + COMPARE_SCALAR_FIELD(options); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 90dfac2cb1..87f5e95827 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10478,7 +10478,9 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = $3; n->indexname = $4; - n->verbose = $2; + n->options = 0; + if ($2) + n->options |= CLUOPT_VERBOSE; $$ = (Node*)n; } | CLUSTER opt_verbose @@ -10486,7 +10488,9 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = NULL; n->indexname = NULL; - n->verbose = $2; + n->options = 0; + if ($2) + n->options |= CLUOPT_VERBOSE; $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10495,7 +10499,9 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = $5; n->indexname = $3; - n->verbose = $2; + n->options = 0; + if ($2) + n->options |= CLUOPT_VERBOSE; $$ = (Node*)n; } ; diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index b338cb1098..f37a60c1c1 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -19,8 +19,7 @@ extern void cluster(ClusterStmt *stmt, bool isTopLevel); -extern void cluster_rel(Oid tableOid, Oid indexOid, bool recheck, - bool verbose); +extern void cluster_rel(Oid tableOid, Oid indexOid, 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); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 9a5d91a198..7855cff30d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3112,12 +3112,18 @@ typedef struct AlterSystemStmt * Cluster Statement (support pbrown's cluster index implementation) * ---------------------- */ +typedef enum ClusterOption +{ + CLUOPT_RECHECK, /* recheck relation state */ + CLUOPT_VERBOSE /* print progress info */ +} ClusterOption; + typedef struct ClusterStmt { NodeTag type; RangeVar *relation; /* relation being indexed, or NULL if all */ char *indexname; /* original index defined */ - bool verbose; /* print progress info */ + int options; /* OR of ClusterOption flags */ } ClusterStmt; /* ---------------------- diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index ed68cc4085..9fe950b29d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -330,6 +330,7 @@ ClosePortalStmt ClosePtrType Clump ClusterInfo +ClusterOption ClusterStmt CmdType CoalesceExpr