On Wed, Apr 3, 2019 at 1:17 PM Kyotaro HORIGUCHI
<horiguchi.kyot...@lab.ntt.co.jp> wrote:
>
> At Wed, 3 Apr 2019 12:10:03 +0900, Masahiko Sawada <sawada.m...@gmail.com> 
> wrote in <cad21aobkahka5sav5n6vvoks9qpmrwrbdyngp8s7m0sspd0...@mail.gmail.com>
> > > And in the following part:
> > >
> > > +       /* Set index cleanup option based on reloptions */
> > > +       if (params->index_cleanup == VACUUM_OPTION_DEFAULT)
> > > +       {
> > > +               if (onerel->rd_options == NULL ||
> > > +                       ((StdRdOptions *) 
> > > onerel->rd_options)->vacuum_index_cleanup)
> > > +                       params->index_cleanup = VACUUM_OPTION_ENABLED;
> > > +               else
> > > +                       params->index_cleanup = VACUUM_OPTION_DISABLED;
> > > +       }
> > > +
> > >
> > > The option should not be false while VACUUM FULL,
> >
> > I think that we need to complain only when INDEX_CLEANUP option is
> > disabled by an explicit option on the VACUUM command and FULL option
> > is specified. It's no problem when vacuum_index_cleanup is false and
> > FULL option is true. Since internally we don't use index cleanup when
> > vacuum full I guess that we don't need to require index_cleanup being
> > always true even when full option is specified.
>
> I know it's safe. It's just about integrity of option values. So
> I don't insist on that.
>
> > > and maybe we
> > > should complain in WARNING or NOTICE that the relopt is ignored.
> >
> > I think when users want to control index cleanup behavior manually
> > they specify INDEX_CLEANUP option on the VACUUM command. So it seems
> > to me that overwriting a reloption by an explicit option would be a
> > natural behavior. I'm concerned that these message would rather
> > confuse users.
>
> If it "cannot be specified with FULL", it seems strange that it's
> safe being specified by reloptions.
>
> I'm rather thinking that INDEX_CLEANUP = false is ignorable even
> being specified with FULL option, aand DISABLE_PAGE_SKIPPING for
> VACUUM FULL shuld be ignored since VACUUM FULL doesn't skip pages
> in the first place.
>
> Couldn't we silence the DISABLE_PAGE_SKIPPING & FULL case instead
> of complaining about INDEX_CLEANUP & FULL? If so, I feel just
> ignoring the relopt cases is reasonable.

Agreed with being silent even when INDEX_CLEANUP/vacuum_index_cleanup
= false and FULL = true. For  DISABLE_PAGE_SKIPPING, it should be a
separate patch and changes the existing behavior. Maybe need other
discussion.

For VacOptTernaryValue part, I've incorporated the review comments but
left the new enum type since it seems to be more straightforward for
now. I might change that if there are other way.

Attached the updated version patches including the
DISABLE_PAGE_SKIPPING part (0003).

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From 48ad0ab24084833b888b9a0f5a7ce1a55fb24775 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Thu, 7 Mar 2019 09:45:11 +0900
Subject: [PATCH v13 1/3] Add INDEX_CLEANUP option to VACUUM command

If this option is false, VACUUM does HOT-pruning for live tuples but
doesn't remove dead tuples completely and disables index vacuum.

vacrelstats->dead_tuples could have tuples that became dead after
checked at a HOT-pruning time, which are not marked as dead. Per
discussion on pgsql-hackers We normally records and remove them but
with this option we don't process and leave for the next vacuum for
simplifing the code. That's okay because it's very rare condition and
those tuples will be processed by the next vacuum.
---
 doc/src/sgml/ref/create_table.sgml     | 15 +++++++
 doc/src/sgml/ref/vacuum.sgml           | 23 ++++++++++
 src/backend/access/common/reloptions.c | 13 +++++-
 src/backend/access/heap/vacuumlazy.c   | 80 ++++++++++++++++++++++++++--------
 src/backend/commands/vacuum.c          | 28 ++++++++++++
 src/backend/postmaster/autovacuum.c    |  1 +
 src/bin/psql/tab-complete.c            |  6 ++-
 src/include/commands/vacuum.h          | 15 +++++++
 src/include/utils/rel.h                |  1 +
 src/test/regress/expected/vacuum.out   |  9 ++++
 src/test/regress/sql/vacuum.sql        | 10 +++++
 11 files changed, 181 insertions(+), 20 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 0fcbc66..910db52 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1378,6 +1378,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
    </varlistentry>
 
    <varlistentry>
+    <term><literal>vacuum_index_cleanup</literal> (<type>boolean</type>)</term>
+    <listitem>
+     <para>
+      Enables or disables index cleanup when <command>VACUUM</command> is
+      run on this table.  The default value is <literal>true</literal>.
+      Disabling index cleanup can speed of <command>VACUUM</command> very
+      significantly, but may also lead to severely bloated indexes if table
+      modifications are frequent.  The <literal>INDEX_CLEANUP</literal>
+      parameter to <xref linkend="sql-vacuum"/>, if specified, overrides
+      the value of this option.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
     <term><literal>autovacuum_vacuum_threshold</literal>, <literal>toast.autovacuum_vacuum_threshold</literal> (<type>integer</type>)</term>
     <listitem>
      <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 906d0c2..643e97b 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -32,6 +32,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     ANALYZE [ <replaceable class="parameter">boolean</replaceable> ]
     DISABLE_PAGE_SKIPPING [ <replaceable class="parameter">boolean</replaceable> ]
     SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
+    INDEX_CLEANUP [ <replaceable class="parameter">boolean</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -182,6 +183,28 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
    </varlistentry>
 
    <varlistentry>
+    <term><literal>INDEX_CLEANUP</literal></term>
+    <listitem>
+     <para>
+      Specifies that <command>VACUUM</command> should attempt to remove
+      index entries pointing to dead tuples.  This is normally the desired
+      behavior and is the default unless the
+      <literal>vacuum_index_cleanup</literal> option has been set to false
+      for the table to be vacuumed.  Setting this option to false may be
+      useful when it is necessary to make vacuum run as quickly as possible,
+      for example to avoid imminent transaction ID wraparound
+      (see <xref linkend="vacuum-for-wraparound"/>).  However, if index
+      cleanup is not performed regularly, performance may suffer, because
+      as the table is modified, indexes will accumulate dead line pointers
+      and the table itself will accumulate dead line pointers that cannot be
+      removed until index cleanup is completed.  This option has no effect
+      for tables that do not have an index and cannot be used in conjunction
+      with <literal>FULL</literal> option.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
      <para>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index b58a1f7..e2c0de3 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -138,6 +138,15 @@ static relopt_bool boolRelOpts[] =
 		},
 		false
 	},
+	{
+		{
+			"vacuum_index_cleanup",
+			"Enables index vacuuming and index cleanup",
+			RELOPT_KIND_HEAP,
+			ShareUpdateExclusiveLock
+		},
+		true
+	},
 	/* list terminator */
 	{{NULL}}
 };
@@ -1388,7 +1397,9 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		{"parallel_workers", RELOPT_TYPE_INT,
 		offsetof(StdRdOptions, parallel_workers)},
 		{"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)}
+		offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)},
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(StdRdOptions, vacuum_index_cleanup)}
 	};
 
 	options = parseRelOptions(reloptions, validate, kind, &numoptions);
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index b5b464e..5c5ddce 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -112,8 +112,8 @@
 
 typedef struct LVRelStats
 {
-	/* hasindex = true means two-pass strategy; false means one-pass */
-	bool		hasindex;
+	/* useindex = true means two-pass strategy; false means one-pass */
+	bool		useindex;
 	/* Overall statistics about rel */
 	BlockNumber old_rel_pages;	/* previous value of pg_class.relpages */
 	BlockNumber rel_pages;		/* total number of pages */
@@ -125,6 +125,8 @@ typedef struct LVRelStats
 	double		new_rel_tuples; /* new estimated total # of tuples */
 	double		new_live_tuples;	/* new estimated total # of live tuples */
 	double		new_dead_tuples;	/* new estimated total # of dead tuples */
+	double		nleft_dead_tuples;	/* # of dead tuples we left */
+	double		nleft_dead_itemids;	/* # of dead item pointers we left */
 	BlockNumber pages_removed;
 	double		tuples_deleted;
 	BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -209,6 +211,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 	MultiXactId new_min_multi;
 
 	Assert(params != NULL);
+	Assert(params->index_cleanup != VACOPT_TERNARY_DEFAULT);
 
 	/* measure elapsed time iff autovacuum logging requires it */
 	if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
@@ -275,7 +278,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 
 	/* Open all indexes of the relation */
 	vac_open_indexes(onerel, RowExclusiveLock, &nindexes, &Irel);
-	vacrelstats->hasindex = (nindexes > 0);
+	vacrelstats->useindex = (nindexes > 0 &&
+							 params->index_cleanup == VACOPT_TERNARY_ENABLED);
 
 	/* Do the vacuuming */
 	lazy_scan_heap(onerel, params->options, vacrelstats, Irel, nindexes, aggressive);
@@ -349,7 +353,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 						new_rel_pages,
 						new_live_tuples,
 						new_rel_allvisible,
-						vacrelstats->hasindex,
+						nindexes > 0,
 						new_frozen_xid,
 						new_min_multi,
 						false);
@@ -419,6 +423,12 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 							 vacrelstats->new_rel_tuples,
 							 vacrelstats->new_dead_tuples,
 							 OldestXmin);
+			if (vacrelstats->nleft_dead_tuples > 0 ||
+				vacrelstats->nleft_dead_itemids > 0)
+				appendStringInfo(&buf,
+								 _("%.0f tuples and %.0f item identifiers are left as dead.\n"),
+								 vacrelstats->nleft_dead_tuples,
+								 vacrelstats->nleft_dead_itemids);
 			appendStringInfo(&buf,
 							 _("buffer usage: %d hits, %d misses, %d dirtied\n"),
 							 VacuumPageHit,
@@ -501,7 +511,10 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 				live_tuples,	/* live tuples (reltuples estimate) */
 				tups_vacuumed,	/* tuples cleaned up by vacuum */
 				nkeep,			/* dead-but-not-removable tuples */
-				nunused;		/* unused item pointers */
+				nunused,		/* unused item pointers */
+				nleft_dead_tuples,		/* tuples we left as dead */
+				nleft_dead_itemids;		/* item pointers we left as dead,
+										 * includes nleft_dead_tuples. */
 	IndexBulkDeleteResult **indstats;
 	int			i;
 	PGRUsage	ru0;
@@ -534,6 +547,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 	empty_pages = vacuumed_pages = 0;
 	next_fsm_block_to_vacuum = (BlockNumber) 0;
 	num_tuples = live_tuples = tups_vacuumed = nkeep = nunused = 0;
+	nleft_dead_itemids = nleft_dead_tuples = 0;
 
 	indstats = (IndexBulkDeleteResult **)
 		palloc0(nindexes * sizeof(IndexBulkDeleteResult *));
@@ -1070,7 +1084,16 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 						HeapTupleIsHeapOnly(&tuple))
 						nkeep += 1;
 					else
+					{
 						tupgone = true; /* we can delete the tuple */
+
+						/*
+						 * Since the dead tuples will be not be vacuumed
+						 * and ignored when index cleanup is disabled we
+						 * count them for reporting.
+						 */
+						nleft_dead_tuples++;
+					}
 					all_visible = false;
 					break;
 				case HEAPTUPLE_LIVE:
@@ -1222,15 +1245,32 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 		}
 
 		/*
-		 * If there are no indexes then we can vacuum the page right now
-		 * instead of doing a second scan.
+		 * If there are no indexes we can vacuum the page right now instead of
+		 * doing a second scan. Also we don't do that but forget dead tuples
+		 * when index cleanup is disabled.
 		 */
-		if (nindexes == 0 &&
-			vacrelstats->num_dead_tuples > 0)
+		if (!vacrelstats->useindex && vacrelstats->num_dead_tuples > 0)
 		{
-			/* Remove tuples from heap */
-			lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats, &vmbuffer);
-			has_dead_tuples = false;
+			if (nindexes == 0)
+			{
+				/* Remove tuples from heap if the table has no index */
+				lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats, &vmbuffer);
+				vacuumed_pages++;
+				has_dead_tuples = false;
+			}
+			else
+			{
+				/*
+				 * Here, we have indexes but index cleanup is disabled. Instead of
+				 * vacuuming the dead tuples on the heap, we just forget them.
+				 *
+				 * Note that vacrelstats->dead_tuples could have tuples which
+				 * became dead after HOT-pruning but are not marked dead yet.
+				 * We do not process them because it's a very rare condition, and
+				 * the next vacuum will process them anyway.
+				 */
+				nleft_dead_itemids += vacrelstats->num_dead_tuples;
+			}
 
 			/*
 			 * Forget the now-vacuumed tuples, and press on, but be careful
@@ -1238,7 +1278,6 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			 * valid.
 			 */
 			vacrelstats->num_dead_tuples = 0;
-			vacuumed_pages++;
 
 			/*
 			 * Periodically do incremental FSM vacuuming to make newly-freed
@@ -1364,7 +1403,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 
 	/* save stats for use later */
 	vacrelstats->tuples_deleted = tups_vacuumed;
-	vacrelstats->new_dead_tuples = nkeep;
+	vacrelstats->new_dead_tuples = nkeep + nleft_dead_tuples;
+	vacrelstats->nleft_dead_tuples = nleft_dead_tuples;
+	vacrelstats->nleft_dead_itemids = nleft_dead_itemids;
 
 	/* now we can compute the new value for pg_class.reltuples */
 	vacrelstats->new_live_tuples = vac_estimate_reltuples(onerel,
@@ -1433,8 +1474,11 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 								 PROGRESS_VACUUM_PHASE_INDEX_CLEANUP);
 
 	/* Do post-vacuum cleanup and statistics update for each index */
-	for (i = 0; i < nindexes; i++)
-		lazy_cleanup_index(Irel[i], indstats[i], vacrelstats);
+	if (vacrelstats->useindex)
+	{
+		for (i = 0; i < nindexes; i++)
+			lazy_cleanup_index(Irel[i], indstats[i], vacrelstats);
+	}
 
 	/* If no indexes, make log report that lazy_vacuum_heap would've made */
 	if (vacuumed_pages)
@@ -1465,6 +1509,8 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 									"%u pages are entirely empty.\n",
 									empty_pages),
 					 empty_pages);
+	appendStringInfo(&buf, "%.0f tuples and %.0f item identifiers are left as dead.\n",
+					 nleft_dead_tuples, nleft_dead_itemids);
 	appendStringInfo(&buf, _("%s."), pg_rusage_show(&ru0));
 
 	ereport(elevel,
@@ -2108,7 +2154,7 @@ lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks)
 	autovacuum_work_mem != -1 ?
 	autovacuum_work_mem : maintenance_work_mem;
 
-	if (vacrelstats->hasindex)
+	if (vacrelstats->useindex)
 	{
 		maxtuples = (vac_work_mem * 1024L) / sizeof(ItemPointerData);
 		maxtuples = Min(maxtuples, INT_MAX);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index fd2e47f..1a7291d 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -76,6 +76,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
 				  TransactionId lastSaneFrozenXid,
 				  MultiXactId lastSaneMinMulti);
 static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params);
+static VacOptTernaryValue get_vacopt_ternary_value(DefElem *def);
 
 /*
  * Primary entry point for manual VACUUM and ANALYZE commands
@@ -95,6 +96,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool disable_page_skipping = false;
 	ListCell	*lc;
 
+	/* Set default value */
+	params.index_cleanup = VACOPT_TERNARY_DEFAULT;
+
 	/* Parse options list */
 	foreach(lc, vacstmt->options)
 	{
@@ -120,6 +124,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			full = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "disable_page_skipping") == 0)
 			disable_page_skipping = defGetBoolean(opt);
+		else if (strcmp(opt->defname, "index_cleanup") == 0)
+			params.index_cleanup = get_vacopt_ternary_value(opt);
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1719,6 +1725,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	onerelid = onerel->rd_lockInfo.lockRelId;
 	LockRelationIdForSession(&onerelid, lmode);
 
+	/* Set index cleanup option based on reloptions if not yet */
+	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
+	{
+		if (onerel->rd_options == NULL ||
+			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			params->index_cleanup = VACOPT_TERNARY_ENABLED;
+		else
+			params->index_cleanup = VACOPT_TERNARY_DISABLED;
+	}
+
 	/*
 	 * Remember the relation's TOAST relation for later, if the caller asked
 	 * us to process it.  In VACUUM FULL, though, the toast table is
@@ -1899,3 +1915,15 @@ vacuum_delay_point(void)
 		CHECK_FOR_INTERRUPTS();
 	}
 }
+
+/*
+ * A wrapper function of defGetBoolean().
+ *
+ * This function returns VACOPT_TERNARY_ENABLED and VACOPT_TERNARY_DISABLED
+ * instead of true and false.
+ */
+static VacOptTernaryValue
+get_vacopt_ternary_value(DefElem *def)
+{
+	return defGetBoolean(def) ? VACOPT_TERNARY_ENABLED : VACOPT_TERNARY_DISABLED;
+}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index fa875db..0976029 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2886,6 +2886,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 			(dovacuum ? VACOPT_VACUUM : 0) |
 			(doanalyze ? VACOPT_ANALYZE : 0) |
 			(!wraparound ? VACOPT_SKIP_LOCKED : 0);
+		tab->at_params.index_cleanup = VACOPT_TERNARY_DEFAULT;
 		tab->at_params.freeze_min_age = freeze_min_age;
 		tab->at_params.freeze_table_age = freeze_table_age;
 		tab->at_params.multixact_freeze_min_age = multixact_freeze_min_age;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d34bf86..22576ad 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1039,6 +1039,7 @@ static const char *const table_storage_parameters[] = {
 	"toast.log_autovacuum_min_duration",
 	"toast_tuple_target",
 	"user_catalog_table",
+	"vacuum_index_cleanup",
 	NULL
 };
 
@@ -3443,8 +3444,9 @@ 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");
-		else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED"))
+						  "DISABLE_PAGE_SKIPPING", "SKIP_LOCKED",
+						  "INDEX_CLEANUP");
+		else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|INDEX_CLEANUP"))
 			COMPLETE_WITH("ON", "OFF");
 	}
 	else if (HeadMatches("VACUUM") && TailMatches("("))
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 77086f3..cbd585a 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -149,6 +149,19 @@ typedef enum VacuumOption
 } VacuumOption;
 
 /*
+ * A ternary value used by vacuum parameters.
+ *
+ * DEFAULT value is used to determine the value based on other
+ * configurations, e.g. reloptions.
+ */
+typedef enum VacOptTernaryValue
+{
+	VACOPT_TERNARY_DISABLED = 0,
+	VACOPT_TERNARY_ENABLED,
+	VACOPT_TERNARY_DEFAULT
+} VacOptTernaryValue;
+
+/*
  * Parameters customizing behavior of VACUUM and ANALYZE.
  *
  * Note that at least one of VACOPT_VACUUM and VACOPT_ANALYZE must be set
@@ -167,6 +180,8 @@ typedef struct VacuumParams
 	int			log_min_duration;	/* minimum execution threshold in ms at
 									 * which  verbose logs are activated, -1
 									 * to use default */
+	VacOptTernaryValue index_cleanup;	/* Do index vacuum and cleanup,
+										* default value depends on reloptions */
 } VacuumParams;
 
 /* GUC parameters */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 5402851..89a7fbf 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -266,6 +266,7 @@ typedef struct StdRdOptions
 	AutoVacOpts autovacuum;		/* autovacuum-related options */
 	bool		user_catalog_table; /* use as an additional catalog relation */
 	int			parallel_workers;	/* max number of parallel workers */
+	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 } StdRdOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 07d0703..6ba7cd7 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -80,6 +80,14 @@ CONTEXT:  SQL function "do_analyze" statement 1
 SQL function "wrap_do_analyze" statement 1
 VACUUM FULL vactst;
 VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
+-- INDEX_CLEANUP option
+CREATE TABLE no_index_cleanup (i INT PRIMARY KEY) WITH (vacuum_index_cleanup = false);
+VACUUM (INDEX_CLEANUP FALSE) vaccluster;
+VACUUM (INDEX_CLEANUP FALSE) vactst; -- index cleanup option is ignored if no indexes
+VACUUM (INDEX_CLEANUP FALSE, FREEZE TRUE) vaccluster;
+-- index cleanup option is ignored if VACUUM FULL
+VACUUM (INDEX_CLEANUP TRUE, FULL TRUE) no_index_cleanup;
+VACUUM (FULL TRUE) no_index_cleanup;
 -- partitioned table
 CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
 CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
@@ -136,6 +144,7 @@ ANALYZE (SKIP_LOCKED) vactst;
 DROP TABLE vaccluster;
 DROP TABLE vactst;
 DROP TABLE vacparted;
+DROP TABLE no_index_cleanup;
 -- relation ownership, WARNING logs generated as all are skipped.
 CREATE TABLE vacowned (a int);
 CREATE TABLE vacowned_parted (a int) PARTITION BY LIST (a);
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 81f3822..57e0f35 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -62,6 +62,15 @@ VACUUM FULL vactst;
 
 VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
 
+-- INDEX_CLEANUP option
+CREATE TABLE no_index_cleanup (i INT PRIMARY KEY) WITH (vacuum_index_cleanup = false);
+VACUUM (INDEX_CLEANUP FALSE) vaccluster;
+VACUUM (INDEX_CLEANUP FALSE) vactst; -- index cleanup option is ignored if no indexes
+VACUUM (INDEX_CLEANUP FALSE, FREEZE TRUE) vaccluster;
+-- index cleanup option is ignored if VACUUM FULL
+VACUUM (INDEX_CLEANUP TRUE, FULL TRUE) no_index_cleanup;
+VACUUM (FULL TRUE) no_index_cleanup;
+
 -- partitioned table
 CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
 CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
@@ -107,6 +116,7 @@ ANALYZE (SKIP_LOCKED) vactst;
 DROP TABLE vaccluster;
 DROP TABLE vactst;
 DROP TABLE vacparted;
+DROP TABLE no_index_cleanup;
 
 -- relation ownership, WARNING logs generated as all are skipped.
 CREATE TABLE vacowned (a int);
-- 
2.10.5

From f4f04f94a83a880c36c1371eea6cf4776fa2805f Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 1 Feb 2019 16:02:50 +0100
Subject: [PATCH v13 2/3] Add --disable-index-cleanup option to vacuumdb.

---
 doc/src/sgml/ref/vacuumdb.sgml    | 15 +++++++++++++++
 src/bin/scripts/t/100_vacuumdb.pl |  9 ++++++++-
 src/bin/scripts/vacuumdb.c        | 29 +++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 47d9345..8a824ec 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -118,6 +118,21 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>--disable-index-cleanup</option></term>
+      <listitem>
+       <para>
+        Disable index vacuuming and index cleanup.
+       </para>
+       <note>
+        <para>
+         This option is only available for servers running
+         <productname>PostgreSQL</productname> 12 and later.
+        </para>
+       </note>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-e</option></term>
       <term><option>--echo</option></term>
       <listitem>
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 7f3a9b1..2b7cd18 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -3,7 +3,7 @@ use warnings;
 
 use PostgresNode;
 use TestLib;
-use Test::More tests => 44;
+use Test::More tests => 47;
 
 program_help_ok('vacuumdb');
 program_version_ok('vacuumdb');
@@ -38,6 +38,10 @@ $node->issues_sql_like(
 	qr/statement: VACUUM \(DISABLE_PAGE_SKIPPING\).*;/,
 	'vacuumdb --disable-page-skipping');
 $node->issues_sql_like(
+	[ 'vacuumdb', '--disable-index-cleanup', 'postgres' ],
+	qr/statement: VACUUM \(INDEX_CLEANUP FALSE\).*;/,
+	'vacuumdb --disable-index-cleanup');
+$node->issues_sql_like(
 	[ 'vacuumdb', '--skip-locked', 'postgres' ],
 	qr/statement: VACUUM \(SKIP_LOCKED\).*;/,
 	'vacuumdb --skip-locked');
@@ -48,6 +52,9 @@ $node->issues_sql_like(
 $node->command_fails(
 	[ 'vacuumdb', '--analyze-only', '--disable-page-skipping', 'postgres' ],
 	'--analyze-only and --disable-page-skipping specified together');
+$node->command_fails(
+	[ 'vacuumdb', '--analyze-only', '--disable-index-cleanup', 'postgres' ],
+	'--analyze-only and --disable-index-cleanup specified together');
 $node->command_ok([qw(vacuumdb -Z --table=pg_am dbname=template1)],
 	'vacuumdb with connection string');
 
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 25ff19e..8711f17 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -43,6 +43,7 @@ typedef struct vacuumingOptions
 	bool		full;
 	bool		freeze;
 	bool		disable_page_skipping;
+	bool		disable_index_cleanup;
 	bool		skip_locked;
 	int			min_xid_age;
 	int			min_mxid_age;
@@ -118,6 +119,7 @@ main(int argc, char *argv[])
 		{"skip-locked", no_argument, NULL, 5},
 		{"min-xid-age", required_argument, NULL, 6},
 		{"min-mxid-age", required_argument, NULL, 7},
+		{"disable-index-cleanup", no_argument, NULL, 8},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -242,6 +244,11 @@ main(int argc, char *argv[])
 					exit(1);
 				}
 				break;
+			case 8:
+				{
+					vacopts.disable_index_cleanup = true;
+					break;
+				}
 			default:
 				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 				exit(1);
@@ -286,6 +293,12 @@ main(int argc, char *argv[])
 						 "disable-page-skipping");
 			exit(1);
 		}
+		if (vacopts.disable_index_cleanup)
+		{
+			pg_log_error("cannot use the \"%s\" option when performing only analyze",
+						 "disable-index-cleanup");
+			exit(1);
+		}
 		/* allow 'and_analyze' with 'analyze_only' */
 	}
 
@@ -414,6 +427,14 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 		exit(1);
 	}
 
+	if (vacopts->disable_index_cleanup && PQserverVersion(conn) < 120000)
+	{
+		PQfinish(conn);
+		pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL 12",
+					 "disable-index-cleanup");
+		exit(1);
+	}
+
 	if (vacopts->skip_locked && PQserverVersion(conn) < 120000)
 	{
 		PQfinish(conn);
@@ -864,6 +885,13 @@ prepare_vacuum_command(PQExpBuffer sql, int serverVersion,
 				appendPQExpBuffer(sql, "%sDISABLE_PAGE_SKIPPING", sep);
 				sep = comma;
 			}
+			if (vacopts->disable_index_cleanup)
+			{
+				/* INDEX_CLEANUP is supported since 12 */
+				Assert(serverVersion >= 120000);
+				appendPQExpBuffer(sql, "%sINDEX_CLEANUP FALSE", sep);
+				sep = comma;
+			}
 			if (vacopts->skip_locked)
 			{
 				/* SKIP_LOCKED is supported since v12 */
@@ -1216,6 +1244,7 @@ help(const char *progname)
 	printf(_("  -a, --all                       vacuum all databases\n"));
 	printf(_("  -d, --dbname=DBNAME             database to vacuum\n"));
 	printf(_("      --disable-page-skipping     disable all page-skipping behavior\n"));
+	printf(_("      --disable-index-cleanup     disable index vacuuming and index cleanup\n"));
 	printf(_("  -e, --echo                      show the commands being sent to the server\n"));
 	printf(_("  -f, --full                      do full vacuuming\n"));
 	printf(_("  -F, --freeze                    freeze row transaction information\n"));
-- 
2.10.5

From ae509e0477130b2ccd9fcf1bcc66b91a463ee995 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Wed, 3 Apr 2019 14:23:30 +0900
Subject: [PATCH v13 3/3] Do not complain even when DISABLE_PAGE_SKIPPING and
 FULL are true

---
 src/backend/commands/vacuum.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 1a7291d..34c761e 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -257,15 +257,6 @@ vacuum(List *relations, VacuumParams *params,
 						stmttype)));
 
 	/*
-	 * Sanity check DISABLE_PAGE_SKIPPING option.
-	 */
-	if ((params->options & VACOPT_FULL) != 0 &&
-		(params->options & VACOPT_DISABLE_PAGE_SKIPPING) != 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL")));
-
-	/*
 	 * Send info about dead objects to the statistics collector, unless we are
 	 * in autovacuum --- autovacuum.c does this for itself.
 	 */
-- 
2.10.5

Reply via email to