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

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 0112a87224129b4261c6ee19b937a9a6d4d409eb..68be4709771487a91ced14a0f23df9968d847fbd 100644 (file)
--- 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 bd0f04c7e24e14779928a8bad0a1377b75768902..5736f12b8f7b27ef1334390db3e50b4de810ec5a 100644 (file)
--- 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 96836ef19cd978dae1a8e5ed824468c380c819fe..17b650b8cb9e0341c31e9d6582300d7ee3c60fa2 100644 (file)
--- 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 6a971d0141a14dc620189afd4a2c2def62682e8b..378f2facb84329d5f55297cf6ec04970467f3673 100644 (file)
--- 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 90dfac2cb156ef1490f4e1ebbb31fac5a45a650d..87f5e958276999746a9edc0ae0d2513fa4043230 100644 (file)
--- 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 b338cb10988ddf5005c6e2eb4f1af72fd5bcd43b..f37a60c1c14fee06f9c40a7c469e89a82bf36ac4 100644 (file)
--- 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 9a5d91a198b5b9e9df5a48d8ae9034f5b2d3d2f2..7855cff30d28473ea6807d91895fe3ebf5796ded 100644 (file)
--- 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 ed68cc4085e008bbf393ab2b820645740192454f..9fe950b29dbb81e4bda9f2529f0b2849149ba43c 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -330,6 +330,7 @@ ClosePortalStmt
 ClosePtrType
 Clump
 ClusterInfo
+ClusterOption
 ClusterStmt
 CmdType
 CoalesceExpr