From 091e1f07f758c110536c6a53d2f70f892212ed18 Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huinker@gmail.com>
Date: Fri, 8 Nov 2024 12:27:50 -0500
Subject: [PATCH v37 11/11] Add --force-analyze to vacuumdb.

The vacuumdb options of --analyze-in-stages and --analyze-only are often
used after a restore from a dump or a pg_upgrade to quickly rebuild
stats on a databse.

However, now that stats are imported in most (but not all) cases,
running either of these commands will be at least partially redundant,
and will overwrite the stats that were just imported, which is a big
POLA violation.

We could add a new option such as --analyze-missing-in-stages, but that
wouldn't help the userbase that grown accustomed to running
--analyze-in-stages after an upgrade.

The least-bad option to handle the situation is to change the behavior
of --analyze-only and --analyze-in-stages to only analyze tables which
were missing stats before the vacuumdb started, but offer the
--force-analyze flag to restore the old behavior for those who truly
wanted it.
---
 src/bin/scripts/t/100_vacuumdb.pl |  6 +-
 src/bin/scripts/vacuumdb.c        | 91 ++++++++++++++++++++++++-------
 doc/src/sgml/ref/vacuumdb.sgml    | 48 ++++++++++++++++
 3 files changed, 125 insertions(+), 20 deletions(-)

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index ccb7711af4..a3fcfda5ee 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -127,9 +127,13 @@ $node->issues_sql_like(
 	qr/statement: VACUUM \(SKIP_DATABASE_STATS, ANALYZE\) public.vactable\(a, b\);/,
 	'vacuumdb --analyze with complete column list');
 $node->issues_sql_like(
+	[ 'vacuumdb', '--analyze-only', '--force-analyze', '--table', 'vactable(b)', 'postgres' ],
+	qr/statement: ANALYZE public.vactable\(b\);/,
+	'vacuumdb --analyze-only --force-analyze with partial column list');
+$node->issues_sql_unlike(
 	[ 'vacuumdb', '--analyze-only', '--table', 'vactable(b)', 'postgres' ],
 	qr/statement: ANALYZE public.vactable\(b\);/,
-	'vacuumdb --analyze-only with partial column list');
+	'vacuumdb --analyze-only --force-analyze with partial column list skipping vacuumed tables');
 $node->command_checks_all(
 	[ 'vacuumdb', '--analyze', '--table', 'vacview', 'postgres' ],
 	0,
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 28506ca0c2..30804af75d 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -25,6 +25,7 @@
 #include "fe_utils/query_utils.h"
 #include "fe_utils/simple_list.h"
 #include "fe_utils/string_utils.h"
+#include "pqexpbuffer.h"
 
 
 /* vacuum options controlled by user flags */
@@ -47,6 +48,8 @@ typedef struct vacuumingOptions
 	bool		process_main;
 	bool		process_toast;
 	bool		skip_database_stats;
+	bool		analyze_in_stages;
+	bool		force_analyze;
 	char	   *buffer_usage_limit;
 } vacuumingOptions;
 
@@ -75,7 +78,6 @@ static void vacuum_one_database(ConnParams *cparams,
 
 static void vacuum_all_databases(ConnParams *cparams,
 								 vacuumingOptions *vacopts,
-								 bool analyze_in_stages,
 								 SimpleStringList *objects,
 								 int concurrentCons,
 								 const char *progname, bool echo, bool quiet);
@@ -140,6 +142,7 @@ main(int argc, char *argv[])
 		{"no-process-toast", no_argument, NULL, 11},
 		{"no-process-main", no_argument, NULL, 12},
 		{"buffer-usage-limit", required_argument, NULL, 13},
+		{"force-analyze", no_argument, NULL, 14},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -156,7 +159,6 @@ main(int argc, char *argv[])
 	bool		echo = false;
 	bool		quiet = false;
 	vacuumingOptions vacopts;
-	bool		analyze_in_stages = false;
 	SimpleStringList objects = {NULL, NULL};
 	int			concurrentCons = 1;
 	int			tbl_count = 0;
@@ -170,6 +172,8 @@ main(int argc, char *argv[])
 	vacopts.do_truncate = true;
 	vacopts.process_main = true;
 	vacopts.process_toast = true;
+	vacopts.force_analyze = false;
+	vacopts.analyze_in_stages = false;
 
 	pg_logging_init(argv[0]);
 	progname = get_progname(argv[0]);
@@ -251,7 +255,7 @@ main(int argc, char *argv[])
 				maintenance_db = pg_strdup(optarg);
 				break;
 			case 3:
-				analyze_in_stages = vacopts.analyze_only = true;
+				vacopts.analyze_in_stages = vacopts.analyze_only = true;
 				break;
 			case 4:
 				vacopts.disable_page_skipping = true;
@@ -287,6 +291,9 @@ main(int argc, char *argv[])
 			case 13:
 				vacopts.buffer_usage_limit = escape_quotes(optarg);
 				break;
+			case 14:
+				vacopts.force_analyze = true;
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -372,6 +379,14 @@ main(int argc, char *argv[])
 		pg_fatal("cannot use the \"%s\" option with the \"%s\" option",
 				 "buffer-usage-limit", "full");
 
+	/*
+	 * --force-analyze is only valid when used with --analyze-only, -analyze,
+	 * or --analyze-in-stages
+	 */
+	if (vacopts.force_analyze && !vacopts.analyze_only && !vacopts.analyze_in_stages)
+		pg_fatal("can only use the \"%s\" option with \"%s\" or \"%s\"",
+				 "--force-analyze", "-Z/--analyze-only", "--analyze-in-stages");
+
 	/* fill cparams except for dbname, which is set below */
 	cparams.pghost = host;
 	cparams.pgport = port;
@@ -390,7 +405,6 @@ main(int argc, char *argv[])
 		cparams.dbname = maintenance_db;
 
 		vacuum_all_databases(&cparams, &vacopts,
-							 analyze_in_stages,
 							 &objects,
 							 concurrentCons,
 							 progname, echo, quiet);
@@ -413,20 +427,27 @@ main(int argc, char *argv[])
 		}
 
 		cparams.dbname = dbname;
-		stage = (analyze_in_stages) ? 0 : ANALYZE_NO_STAGE;
+		stage = (vacopts.analyze_in_stages) ? 0 : ANALYZE_NO_STAGE;
 
 		conn = connectDatabase(&cparams, progname, echo, false, true);
 		check_conn_options(conn, &vacopts);
 		print_processing_notice(conn, stage, progname, quiet);
 		found_objects = generate_catalog_list(conn, &vacopts, &objects, echo, &ntup);
+		vacuum_one_database(&cparams, &vacopts, stage,
+							&objects,
+							concurrentCons,
+							conn,
+							found_objects,
+							ntup,
+							progname, echo, quiet);
 
-		if (analyze_in_stages)
+		if (stage != ANALYZE_NO_STAGE)
 		{
-			for (stage = 0; stage < ANALYZE_NUM_STAGES; stage++)
+			for (stage = 1; stage < ANALYZE_NUM_STAGES; stage++)
 			{
 				/* the last pass disconnected the conn */
-				if (stage > 0)
-					conn = connectDatabase(&cparams, progname, echo, false, true);
+				conn = connectDatabase(&cparams, progname, echo, false, true);
+				print_processing_notice(conn, stage, progname, quiet);
 
 				vacuum_one_database(&cparams, &vacopts, stage,
 									&objects,
@@ -437,14 +458,6 @@ main(int argc, char *argv[])
 									progname, echo, quiet);
 			}
 		}
-		else
-			vacuum_one_database(&cparams, &vacopts, stage,
-								&objects,
-								concurrentCons,
-								conn,
-								found_objects,
-								ntup,
-								progname, echo, quiet);
 	}
 
 	exit(0);
@@ -763,6 +776,47 @@ generate_catalog_list(PGconn *conn,
 						  vacopts->min_mxid_age);
 	}
 
+	/*
+	 * If this query is for an analyze-only or analyze-in-stages, two
+	 * upgrade-centric operations, and force-analyze is NOT set, then
+	 * exclude any relations that already have their full compliment
+	 * of attribute stats and extended stats.
+	 *
+	 *
+	 */
+	if ((vacopts->analyze_only || vacopts->analyze_in_stages) &&
+		!vacopts->force_analyze)
+	{
+		/*
+		 * The pg_class in question has no pg_statistic rows representing
+		 * user-visible columns that lack a corresponding pg_statitic row.
+		 * Currently no differentiation is made for whether the
+		 * pg_statistic.stainherit is true or false.
+		 */
+		appendPQExpBufferStr(&catalog_query,
+							 " AND NOT EXISTS (\n"
+							 " SELECT NULL FROM pg_catalog.pg_attribute AS a\n"
+							 " WHERE a.attrelid OPERATOR(pg_catalog.=) c.oid\n"
+							 " AND a.attnum OPERATOR(pg_catalog.>) 0 AND NOT a.attisdropped\n"
+							 " AND NOT EXISTS (\n"
+							 " SELECT NULL FROM pg_catalog.pg_statistic AS s\n"
+							 " WHERE s.starelid OPERATOR(pg_catalog.=) c.oid\n"
+							 " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum))\n");
+
+		/*
+		 * The pg_class entry has no pg_statistic_ext rows that lack a corresponding
+		 * pg_statistic_ext_data row. Currently no differentiation is made for whether
+		 * pg_statistic_exta_data.stxdinherit is true or false.
+		 */
+		appendPQExpBufferStr(&catalog_query,
+							 " AND NOT EXISTS (\n"
+							 " SELECT NULL FROM pg_catalog.pg_statistic_ext AS e\n"
+							 " WHERE e.stxrelid OPERATOR(pg_catalog.=) c.oid\n"
+							 " AND NOT EXISTS (\n"
+							 " SELECT NULL FROM pg_catalog.pg_statistic_ext_data AS d\n"
+							 " WHERE d.stxoid OPERATOR(pg_catalog.=) e.oid))\n");
+	}
+
 	/*
 	 * Execute the catalog query.  We use the default search_path for this
 	 * query for consistency with table lookups done elsewhere by the user.
@@ -959,7 +1013,6 @@ finish:
 static void
 vacuum_all_databases(ConnParams *cparams,
 					 vacuumingOptions *vacopts,
-					 bool analyze_in_stages,
 					 SimpleStringList *objects,
 					 int concurrentCons,
 					 const char *progname, bool echo, bool quiet)
@@ -972,7 +1025,7 @@ vacuum_all_databases(ConnParams *cparams,
 	SimpleStringList  **found_objects;
 	int				   *num_tuples;
 
-	stage = (analyze_in_stages) ? 0 : ANALYZE_NO_STAGE;
+	stage = (vacopts->analyze_in_stages) ? 0 : ANALYZE_NO_STAGE;
 
 	conn = connectMaintenanceDatabase(cparams, progname, echo);
 	result = executeQuery(conn,
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 66fccb30a2..00ec927606 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -425,6 +425,12 @@ PostgreSQL documentation
        <para>
         Only calculate statistics for use by the optimizer (no vacuum).
        </para>
+       <para>
+        By default, this operation excludes relations that already have
+        statistics generated. If the option <option>--force-analyze</option>
+        is also specified, then relations with existing stastistics are not
+        excluded.
+       </para>
       </listitem>
      </varlistentry>
 
@@ -439,6 +445,47 @@ PostgreSQL documentation
         to produce usable statistics faster, and subsequent stages build the
         full statistics.
        </para>
+       <para>
+        This option is intended to be run after a <command>pg_upgrade</command>
+        to generate statistics for relations that have no stistatics or incomplete
+        statistics (such as those with extended statistics objects, which are not
+        imported on upgrade).
+       </para>
+       <para>
+        If the option <option>--force-analyze</option> is also specified, then
+        relations with existing stastistics are not excluded.
+        This option is only useful to analyze a database that currently has
+        no statistics or has wholly incorrect ones, such as if it is newly
+        populated from a restored dump.
+        Be aware that running with this option combinationin a database with
+        existing statistics may cause the query optimizer choices to become
+        transiently worse due to the low statistics targets of the early
+        stages.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><option>--force-analyze</option></term>
+      <listitem>
+       <para>
+        This option can only be used if either <option>--analyze-only</option>
+        or <option>--analyze-in-stages</option> is specified. It modifies those
+        options to not filter out relations that already have statistics.
+       </para>
+       <para>
+
+        Only calculate statistics for use by the optimizer (no vacuum),
+        like <option>--analyze-only</option>.  Run three
+        stages of analyze; the first stage uses the lowest possible statistics
+        target (see <xref linkend="guc-default-statistics-target"/>)
+        to produce usable statistics faster, and subsequent stages build the
+        full statistics.
+       </para>
+
+       <para>
+        This option was created
+       </para>
 
        <para>
         This option is only useful to analyze a database that currently has
@@ -452,6 +499,7 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+
      <varlistentry>
        <term><option>-?</option></term>
        <term><option>--help</option></term>
-- 
2.47.1

