From a3dce99b48a6c97c1f72da63f38adbfcc22d462d Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Sun, 1 Mar 2015 20:45:06 +0900
Subject: [PATCH] Move freeze parameters of VacuumStmt into a separate
 structure

VACOPT_FREEZE was being limited in the query parser and still be used
in a set of assertions at the beginning of vacuum(). On the contrary,
the set of parameters controlling if FREEZE is used was being set in the
query parser but not used in the same set of assertions, leading to
confusion regarding both aspects of VacuumStmt.

This commit fixes this inconsistency by moving all the freeze parameters
to a dedicated structure used by autovacuum and by setting unconditionally
VACOPT_FREEZE for all the code paths of the query parser, making the whole
control of FREEZE more consistent in the autovacuum code path as well as
in the query parser.

Per an idea from Stephen Frost.

to be used in the grammar with and still use it in the set of assertions
---
 src/backend/commands/vacuum.c       | 18 +++++++------
 src/backend/commands/vacuumlazy.c   | 38 +++++++++++++++++++++++++---
 src/backend/nodes/copyfuncs.c       |  4 ---
 src/backend/nodes/equalfuncs.c      |  4 ---
 src/backend/parser/gram.y           | 50 +++++--------------------------------
 src/backend/postmaster/autovacuum.c | 17 +++++++------
 src/backend/tcop/utility.c          |  2 +-
 src/include/commands/vacuum.h       | 19 +++++++++++---
 src/include/nodes/parsenodes.h      | 14 +++--------
 9 files changed, 81 insertions(+), 85 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2f3f79d..c2234e6 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -71,13 +71,16 @@ static void vac_truncate_clog(TransactionId frozenXID,
 				  MultiXactId minMulti,
 				  TransactionId lastSaneFrozenXid,
 				  MultiXactId lastSaneMinMulti);
-static bool vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
-		   bool for_wraparound);
+static bool vacuum_rel(Oid relid, VacuumStmt *vacstmt, VacuumParams *params,
+					   bool do_toast, bool for_wraparound);
 
 
 /*
  * Primary entry point for VACUUM and ANALYZE commands.
  *
+ * params contains a set of parameters used by autovacuum to customize the
+ * behavior of process.
+ *
  * relid is normally InvalidOid; if it is not, then it provides the relation
  * OID to be processed, and vacstmt->relation is ignored.  (The non-invalid
  * case is currently only used by autovacuum.)
@@ -98,7 +101,7 @@ static bool vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
  * at transaction commit.
  */
 void
-vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
+vacuum(VacuumStmt *vacstmt, VacuumParams *params, Oid relid, bool do_toast,
 	   BufferAccessStrategy bstrategy, bool for_wraparound, bool isTopLevel)
 {
 	const char *stmttype;
@@ -247,7 +250,7 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
 
 			if (vacstmt->options & VACOPT_VACUUM)
 			{
-				if (!vacuum_rel(relid, vacstmt, do_toast, for_wraparound))
+				if (!vacuum_rel(relid, vacstmt, params, do_toast, for_wraparound))
 					continue;
 			}
 
@@ -1113,7 +1116,8 @@ vac_truncate_clog(TransactionId frozenXID,
  *		At entry and exit, we are not inside a transaction.
  */
 static bool
-vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
+vacuum_rel(Oid relid, VacuumStmt *vacstmt, VacuumParams *params,
+		   bool do_toast, bool for_wraparound)
 {
 	LOCKMODE	lmode;
 	Relation	onerel;
@@ -1316,7 +1320,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
 					(vacstmt->options & VACOPT_VERBOSE) != 0);
 	}
 	else
-		lazy_vacuum_rel(onerel, vacstmt, vac_strategy);
+		lazy_vacuum_rel(onerel, vacstmt, params, vac_strategy);
 
 	/* Roll back any GUC changes executed by index functions */
 	AtEOXact_GUC(false, save_nestlevel);
@@ -1342,7 +1346,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
 	 * totally unimportant for toast relations.
 	 */
 	if (toast_relid != InvalidOid)
-		vacuum_rel(toast_relid, vacstmt, false, for_wraparound);
+		vacuum_rel(toast_relid, vacstmt, params, false, for_wraparound);
 
 	/*
 	 * Now release the session-level lock on the master table.
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 7d9e49e..adab3b7 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -169,7 +169,7 @@ static bool heap_page_is_all_visible(Relation rel, Buffer buf,
  *		and locked the relation.
  */
 void
-lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
+lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, VacuumParams *params,
 				BufferAccessStrategy bstrategy)
 {
 	LVRelStats *vacrelstats;
@@ -192,6 +192,10 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 	double		new_live_tuples;
 	TransactionId new_frozen_xid;
 	MultiXactId new_min_multi;
+	int			freeze_min_age,
+				freeze_table_age,
+				multixact_freeze_min_age,
+				multixact_freeze_table_age;
 
 	/* measure elapsed time iff autovacuum logging requires it */
 	if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -207,10 +211,36 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 
 	vac_strategy = bstrategy;
 
+	/* set up FREEZE parameters */
+	if (params != NULL)
+	{
+		freeze_min_age = params->freeze_min_age;
+		freeze_table_age = params->freeze_table_age;
+		multixact_freeze_min_age = params->multixact_freeze_min_age;
+		multixact_freeze_table_age = params->multixact_freeze_table_age;
+	}
+	else
+	{
+		if (vacstmt->options & VACOPT_FREEZE)
+		{
+			freeze_min_age = 0;
+			freeze_table_age = 0;
+			multixact_freeze_min_age = 0;
+			multixact_freeze_table_age = 0;
+		}
+		else
+		{
+			freeze_min_age = -1;
+			freeze_table_age = -1;
+			multixact_freeze_min_age = -1;
+			multixact_freeze_table_age = -1;
+		}
+	}
+
 	vacuum_set_xid_limits(onerel,
-						  vacstmt->freeze_min_age, vacstmt->freeze_table_age,
-						  vacstmt->multixact_freeze_min_age,
-						  vacstmt->multixact_freeze_table_age,
+						  freeze_min_age, freeze_table_age,
+						  multixact_freeze_min_age,
+						  multixact_freeze_table_age,
 						  &OldestXmin, &FreezeLimit, &xidFullScanLimit,
 						  &MultiXactCutoff, &mxactFullScanLimit);
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 9fe8008..4f0e278 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3292,10 +3292,6 @@ _copyVacuumStmt(const VacuumStmt *from)
 	VacuumStmt *newnode = makeNode(VacuumStmt);
 
 	COPY_SCALAR_FIELD(options);
-	COPY_SCALAR_FIELD(freeze_min_age);
-	COPY_SCALAR_FIELD(freeze_table_age);
-	COPY_SCALAR_FIELD(multixact_freeze_min_age);
-	COPY_SCALAR_FIELD(multixact_freeze_table_age);
 	COPY_NODE_FIELD(relation);
 	COPY_NODE_FIELD(va_cols);
 
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index fe509b0..6b55500 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1507,10 +1507,6 @@ static bool
 _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b)
 {
 	COMPARE_SCALAR_FIELD(options);
-	COMPARE_SCALAR_FIELD(freeze_min_age);
-	COMPARE_SCALAR_FIELD(freeze_table_age);
-	COMPARE_SCALAR_FIELD(multixact_freeze_min_age);
-	COMPARE_SCALAR_FIELD(multixact_freeze_table_age);
 	COMPARE_NODE_FIELD(relation);
 	COMPARE_NODE_FIELD(va_cols);
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 581f7a1..4b9a17f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9068,12 +9068,10 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->options = VACOPT_VACUUM;
 					if ($2)
 						n->options |= VACOPT_FULL;
+					if ($3)
+						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					n->freeze_min_age = $3 ? 0 : -1;
-					n->freeze_table_age = $3 ? 0 : -1;
-					n->multixact_freeze_min_age = $3 ? 0 : -1;
-					n->multixact_freeze_table_age = $3 ? 0 : -1;
 					n->relation = NULL;
 					n->va_cols = NIL;
 					$$ = (Node *)n;
@@ -9084,12 +9082,10 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->options = VACOPT_VACUUM;
 					if ($2)
 						n->options |= VACOPT_FULL;
+					if ($3)
+						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					n->freeze_min_age = $3 ? 0 : -1;
-					n->freeze_table_age = $3 ? 0 : -1;
-					n->multixact_freeze_min_age = $3 ? 0 : -1;
-					n->multixact_freeze_table_age = $3 ? 0 : -1;
 					n->relation = $5;
 					n->va_cols = NIL;
 					$$ = (Node *)n;
@@ -9100,30 +9096,16 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->options |= VACOPT_VACUUM;
 					if ($2)
 						n->options |= VACOPT_FULL;
+					if ($3)
+						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					n->freeze_min_age = $3 ? 0 : -1;
-					n->freeze_table_age = $3 ? 0 : -1;
-					n->multixact_freeze_min_age = $3 ? 0 : -1;
-					n->multixact_freeze_table_age = $3 ? 0 : -1;
 					$$ = (Node *)n;
 				}
 			| VACUUM '(' vacuum_option_list ')'
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_VACUUM | $3;
-					if (n->options & VACOPT_FREEZE)
-					{
-						n->freeze_min_age = n->freeze_table_age = 0;
-						n->multixact_freeze_min_age = 0;
-						n->multixact_freeze_table_age = 0;
-					}
-					else
-					{
-						n->freeze_min_age = n->freeze_table_age = -1;
-						n->multixact_freeze_min_age = -1;
-						n->multixact_freeze_table_age = -1;
-					}
 					n->relation = NULL;
 					n->va_cols = NIL;
 					$$ = (Node *) n;
@@ -9132,18 +9114,6 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_VACUUM | $3;
-					if (n->options & VACOPT_FREEZE)
-					{
-						n->freeze_min_age = n->freeze_table_age = 0;
-						n->multixact_freeze_min_age = 0;
-						n->multixact_freeze_table_age = 0;
-					}
-					else
-					{
-						n->freeze_min_age = n->freeze_table_age = -1;
-						n->multixact_freeze_min_age = -1;
-						n->multixact_freeze_table_age = -1;
-					}
 					n->relation = $5;
 					n->va_cols = $6;
 					if (n->va_cols != NIL)	/* implies analyze */
@@ -9171,10 +9141,6 @@ AnalyzeStmt:
 					n->options = VACOPT_ANALYZE;
 					if ($2)
 						n->options |= VACOPT_VERBOSE;
-					n->freeze_min_age = -1;
-					n->freeze_table_age = -1;
-					n->multixact_freeze_min_age = -1;
-					n->multixact_freeze_table_age = -1;
 					n->relation = NULL;
 					n->va_cols = NIL;
 					$$ = (Node *)n;
@@ -9185,10 +9151,6 @@ AnalyzeStmt:
 					n->options = VACOPT_ANALYZE;
 					if ($2)
 						n->options |= VACOPT_VERBOSE;
-					n->freeze_min_age = -1;
-					n->freeze_table_age = -1;
-					n->multixact_freeze_min_age = -1;
-					n->multixact_freeze_table_age = -1;
 					n->relation = $3;
 					n->va_cols = $4;
 					$$ = (Node *)n;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 77158c1..67c8cdc 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2740,8 +2740,9 @@ static void
 autovacuum_do_vac_analyze(autovac_table *tab,
 						  BufferAccessStrategy bstrategy)
 {
-	VacuumStmt	vacstmt;
-	RangeVar	rangevar;
+	VacuumStmt		vacstmt;
+	VacuumParams	params;
+	RangeVar		rangevar;
 
 	/* Set up command parameters --- use local variables instead of palloc */
 	MemSet(&vacstmt, 0, sizeof(vacstmt));
@@ -2758,18 +2759,20 @@ autovacuum_do_vac_analyze(autovac_table *tab,
 		vacstmt.options |= VACOPT_VACUUM;
 	if (tab->at_doanalyze)
 		vacstmt.options |= VACOPT_ANALYZE;
-	vacstmt.freeze_min_age = tab->at_freeze_min_age;
-	vacstmt.freeze_table_age = tab->at_freeze_table_age;
-	vacstmt.multixact_freeze_min_age = tab->at_multixact_freeze_min_age;
-	vacstmt.multixact_freeze_table_age = tab->at_multixact_freeze_table_age;
 	/* we pass the OID, but might need this anyway for an error message */
 	vacstmt.relation = &rangevar;
 	vacstmt.va_cols = NIL;
 
+	params.freeze_min_age = tab->at_freeze_min_age;
+	params.freeze_table_age = tab->at_freeze_table_age;
+	params.multixact_freeze_min_age = tab->at_multixact_freeze_min_age;
+	params.multixact_freeze_table_age = tab->at_multixact_freeze_table_age;
+
 	/* Let pgstat know what we're doing */
 	autovac_report_activity(tab);
 
-	vacuum(&vacstmt, tab->at_relid, false, bstrategy, tab->at_wraparound, true);
+	vacuum(&vacstmt, &params, tab->at_relid, false, bstrategy,
+		   tab->at_wraparound, true);
 }
 
 /*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 6d26986..8ac7e3b 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -627,7 +627,7 @@ standard_ProcessUtility(Node *parsetree,
 				/* we choose to allow this during "read only" transactions */
 				PreventCommandDuringRecovery((stmt->options & VACOPT_VACUUM) ?
 											 "VACUUM" : "ANALYZE");
-				vacuum(stmt, InvalidOid, true, NULL, false, isTopLevel);
+				vacuum(stmt, NULL, InvalidOid, true, NULL, false, isTopLevel);
 			}
 			break;
 
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 4275484..2c6a5b5 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -130,6 +130,18 @@ typedef struct VacAttrStats
 	int			rowstride;
 } VacAttrStats;
 
+/*
+ * Parameters customizing behavior of a VACUUM or ANALYZE query.
+ */
+typedef struct VacuumParams
+{
+	int		freeze_min_age;		/* min freeze age, or -1 to use default */
+	int		freeze_table_age;       /* age at which to scan whole table */
+	int		multixact_freeze_min_age;       /* min multixact freeze age,
+												 * or -1 to use default */
+	int		multixact_freeze_table_age;     /* multixact age at which to
+											 * scan whole table */
+} VacuumParams;
 
 /* GUC parameters */
 extern PGDLLIMPORT int default_statistics_target;		/* PGDLLIMPORT for
@@ -141,8 +153,9 @@ extern int	vacuum_multixact_freeze_table_age;
 
 
 /* in commands/vacuum.c */
-extern void vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
-	   BufferAccessStrategy bstrategy, bool for_wraparound, bool isTopLevel);
+extern void vacuum(VacuumStmt *vacstmt, VacuumParams *params, Oid relid,
+				   bool do_toast, BufferAccessStrategy bstrategy,
+				   bool for_wraparound, bool isTopLevel);
 extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
 				 int *nindexes, Relation **Irel);
 extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode);
@@ -172,7 +185,7 @@ extern void vacuum_delay_point(void);
 
 /* in commands/vacuumlazy.c */
 extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
-				BufferAccessStrategy bstrategy);
+				VacuumParams *params, BufferAccessStrategy bstrategy);
 
 /* in commands/analyze.c */
 extern void analyze_rel(Oid relid, VacuumStmt *vacstmt,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ac13302..4c8b14c 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2598,9 +2598,7 @@ typedef struct ClusterStmt
  *
  * Even though these are nominally two statements, it's convenient to use
  * just one node type for both.  Note that at least one of VACOPT_VACUUM
- * and VACOPT_ANALYZE must be set in options.  VACOPT_FREEZE is an internal
- * convenience for the grammar and is not examined at runtime --- the
- * freeze_min_age and freeze_table_age fields are what matter.
+ * and VACOPT_ANALYZE must be set in options.
  * ----------------------
  */
 typedef enum VacuumOption
@@ -2615,14 +2613,8 @@ typedef enum VacuumOption
 
 typedef struct VacuumStmt
 {
-	NodeTag		type;
-	int			options;		/* OR of VacuumOption flags */
-	int			freeze_min_age; /* min freeze age, or -1 to use default */
-	int			freeze_table_age;		/* age at which to scan whole table */
-	int			multixact_freeze_min_age;		/* min multixact freeze age,
-												 * or -1 to use default */
-	int			multixact_freeze_table_age;		/* multixact age at which to
-												 * scan whole table */
+	NodeTag			type;
+	int				options;	/* OR of VacuumOption flags */
 	RangeVar   *relation;		/* single table to process, or NULL */
 	List	   *va_cols;		/* list of column names, or NIL for all */
 } VacuumStmt;
-- 
2.3.1

