v2-0004-Always-use-a-catalog-query-to-discover-tables-to-.patch

From 3c2ee6a10ea529a8eacd4cfb8f6754ca708dfa93 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Fri, 4 Jan 2019 22:00:28 +0000
Subject: [PATCH v2 4/6] Always use a catalog query to discover tables to
 process in vacuumdb.

---
 src/bin/scripts/common.c          |   2 +-
 src/bin/scripts/common.h          |   3 +
 src/bin/scripts/t/100_vacuumdb.pl |  12 ++--
 src/bin/scripts/vacuumdb.c        | 144 +++++++++++++++++++++-----------------
 4 files changed, 90 insertions(+), 71 deletions(-)

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 4215bc3d6e..493408d15d 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -265,7 +265,7 @@ executeMaintenanceCommand(PGconn *conn, const char *query, bool echo)
  * finish using them, pg_free(*table).  *columns is a pointer into "spec",
  * possibly to its NUL terminator.
  */
-static void
+void
 split_table_columns_spec(const char *spec, int encoding,
 						 char **table, const char **columns)
 {
diff --git a/src/bin/scripts/common.h b/src/bin/scripts/common.h
index 7c3888cefd..f3a7616bae 100644
--- a/src/bin/scripts/common.h
+++ b/src/bin/scripts/common.h
@@ -48,6 +48,9 @@ extern void executeCommand(PGconn *conn, const char *query,
 extern bool executeMaintenanceCommand(PGconn *conn, const char *query,
 						  bool echo);
 
+extern void split_table_columns_spec(const char *spec, int encoding,
+						 char **table, const char **columns);
+
 extern void appendQualifiedRelation(PQExpBuffer buf, const char *name,
 						PGconn *conn, const char *progname, bool echo);
 
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 5f22323006..a9553698ad 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -15,15 +15,15 @@ $node->start;
 
 $node->issues_sql_like(
 	[ 'vacuumdb', 'postgres' ],
-	qr/statement: VACUUM;/,
+	qr/statement: VACUUM pg_catalog\./,
 	'SQL VACUUM run');
 $node->issues_sql_like(
 	[ 'vacuumdb', '-f', 'postgres' ],
-	qr/statement: VACUUM \(FULL\);/,
+	qr/statement: VACUUM \(FULL\) pg_catalog\./,
 	'vacuumdb -f');
 $node->issues_sql_like(
 	[ 'vacuumdb', '-F', 'postgres' ],
-	qr/statement: VACUUM \(FREEZE\);/,
+	qr/statement: VACUUM \(FREEZE\) pg_catalog\./,
 	'vacuumdb -F');
 $node->issues_sql_like(
 	[ 'vacuumdb', '-zj2', 'postgres' ],
@@ -31,15 +31,15 @@ $node->issues_sql_like(
 	'vacuumdb -zj2');
 $node->issues_sql_like(
 	[ 'vacuumdb', '-Z', 'postgres' ],
-	qr/statement: ANALYZE;/,
+	qr/statement: ANALYZE pg_catalog\./,
 	'vacuumdb -Z');
 $node->issues_sql_like(
 	[ 'vacuumdb', '--disable-page-skipping', 'postgres' ],
-	qr/statement: VACUUM \(DISABLE_PAGE_SKIPPING\);/,
+	qr/statement: VACUUM \(DISABLE_PAGE_SKIPPING\) pg_catalog\./,
 	'vacuumdb --disable-page-skipping');
 $node->issues_sql_like(
 	[ 'vacuumdb', '--skip-locked', 'postgres' ],
-	qr/statement: VACUUM \(SKIP_LOCKED\);/,
+	qr/statement: VACUUM \(SKIP_LOCKED\) pg_catalog\./,
 	'vacuumdb --skip-locked');
 $node->command_fails(
 	[qw|vacuumdb --analyze-only --disable-page-skipping postgres|],
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index bdc4fcfe35..78669a35f4 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -19,6 +19,7 @@
 #include "catalog/pg_class_d.h"
 
 #include "common.h"
+#include "fe_utils/connect.h"
 #include "fe_utils/simple_list.h"
 #include "fe_utils/string_utils.h"
 
@@ -62,9 +63,7 @@ static void vacuum_all_databases(vacuumingOptions *vacopts,
 					 const char *progname, bool echo, bool quiet);
 
 static void prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
-					   vacuumingOptions *vacopts, const char *table,
-					   bool table_pre_qualified,
-					   const char *progname, bool echo);
+					   vacuumingOptions *vacopts, const char *table);
 
 static void run_vacuum_command(PGconn *conn, const char *sql, bool echo,
 				   const char *table, const char *progname, bool async);
@@ -359,13 +358,18 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 					const char *progname, bool echo, bool quiet)
 {
 	PQExpBufferData sql;
+	PQExpBufferData buf;
+	PQExpBufferData catalog_query;
+	PGresult   *res;
 	PGconn	   *conn;
 	SimpleStringListCell *cell;
 	ParallelSlot *slots;
 	SimpleStringList dbtables = {NULL, NULL};
 	int			i;
+	int			ntups;
 	bool		failed = false;
 	bool		parallel = concurrentCons > 1;
+	bool		first_table = true;
 	const char *stage_commands[] = {
 		"SET default_statistics_target=1; SET vacuum_cost_delay=0;",
 		"SET default_statistics_target=10; RESET vacuum_cost_delay;",
@@ -408,58 +412,84 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 		fflush(stdout);
 	}
 
-	/*
-	 * If a table list is not provided and we're using multiple connections,
-	 * prepare the list of tables by querying the catalogs.
-	 */
-	if (parallel && (!tables || !tables->head))
+	initPQExpBuffer(&catalog_query);
+	appendPQExpBuffer(&catalog_query,
+					  "SELECT c.relname, ns.nspname"
+					  " FROM pg_catalog.pg_class c\n"
+					  " JOIN pg_catalog.pg_namespace ns"
+					  " ON c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n"
+					  " LEFT JOIN pg_catalog.pg_class t"
+					  " ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n"
+					  " WHERE c.relkind IN ("
+					  CppAsString2(RELKIND_RELATION) ", "
+					  CppAsString2(RELKIND_MATVIEW) ")\n");
+
+	cell = tables ? tables->head : NULL;
+	while (cell != NULL)
 	{
-		PQExpBufferData buf;
-		PGresult   *res;
-		int			ntups;
-
-		res = executeQuery(conn,
-						   "SELECT c.relname, ns.nspname"
-						   " FROM pg_class c, pg_namespace ns\n"
-						   " WHERE relkind IN ("
-						   CppAsString2(RELKIND_RELATION) ", "
-						   CppAsString2(RELKIND_MATVIEW) ")"
-						   " AND c.relnamespace = ns.oid\n"
-						   " ORDER BY c.relpages DESC;",
-						   progname, echo);
-
-		ntups = PQntuples(res);
-		if (ntups == 0)
+		char	   *just_table;
+		const char *just_columns;
+
+		split_table_columns_spec(cell->val, PQclientEncoding(conn),
+								 &just_table, &just_columns);
+
+		if (first_table)
 		{
-			PQclear(res);
-			PQfinish(conn);
-			return;
+			appendPQExpBuffer(&catalog_query, " AND (\n  c.oid OPERATOR(pg_catalog.=) ");
+			first_table = false;
 		}
+		else
+			appendPQExpBuffer(&catalog_query, "  OR c.oid OPERATOR(pg_catalog.=) ");
 
-		initPQExpBuffer(&buf);
-		for (i = 0; i < ntups; i++)
-		{
-			appendPQExpBufferStr(&buf,
-								 fmtQualifiedId(PQgetvalue(res, i, 1),
-												PQgetvalue(res, i, 0)));
+		appendStringLiteralConn(&catalog_query, just_table, conn);
+		appendPQExpBuffer(&catalog_query, "::pg_catalog.regclass\n");
 
-			simple_string_list_append(&dbtables, buf.data);
-			resetPQExpBuffer(&buf);
-		}
+		pg_free(just_table);
 
-		termPQExpBuffer(&buf);
-		tables = &dbtables;
+		cell = cell->next;
+		if (cell == NULL)
+			appendPQExpBuffer(&catalog_query, " )\n");
+	}
 
-		/*
-		 * If there are more connections than vacuumable relations, we don't
-		 * need to use them all.
-		 */
+	appendPQExpBuffer(&catalog_query, " ORDER BY c.relpages DESC;");
+	executeCommand(conn, "RESET search_path;", progname, echo);
+	res = executeQuery(conn, catalog_query.data, progname, echo);
+	termPQExpBuffer(&catalog_query);
+	PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
+				progname, echo));
+
+	ntups = PQntuples(res);
+	if (ntups == 0)
+	{
+		PQclear(res);
+		PQfinish(conn);
+		return;
+	}
+
+	initPQExpBuffer(&buf);
+	for (i = 0; i < ntups; i++)
+	{
+		appendPQExpBufferStr(&buf,
+							 fmtQualifiedId(PQgetvalue(res, i, 1),
+											PQgetvalue(res, i, 0)));
+
+		simple_string_list_append(&dbtables, buf.data);
+		resetPQExpBuffer(&buf);
+	}
+	termPQExpBuffer(&buf);
+
+	/*
+	 * If there are more connections than vacuumable relations, we don't need
+	 * to use them all.
+	 */
+	if (parallel)
+	{
 		if (concurrentCons > ntups)
 			concurrentCons = ntups;
 		if (concurrentCons <= 1)
 			parallel = false;
-		PQclear(res);
 	}
+	PQclear(res);
 
 	/*
 	 * Setup the database connections. We reuse the connection we already have
@@ -497,10 +527,10 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 
 	initPQExpBuffer(&sql);
 
-	cell = tables ? tables->head : NULL;
+	cell = dbtables.head;
 	do
 	{
-		const char *tabname = cell ? cell->val : NULL;
+		const char *tabname = cell->val;
 		ParallelSlot *free_slot;
 
 		if (CancelRequested)
@@ -533,12 +563,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 		else
 			free_slot = slots;
 
-		/*
-		 * Prepare the vacuum command.  Note that in some cases this requires
-		 * query execution, so be sure to use the free connection.
-		 */
-		prepare_vacuum_command(&sql, free_slot->connection, vacopts, tabname,
-							   tables == &dbtables, progname, echo);
+		prepare_vacuum_command(&sql, free_slot->connection, vacopts, tabname);
 
 		/*
 		 * Execute the vacuum.  If not in parallel mode, this terminates the
@@ -548,8 +573,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 		run_vacuum_command(free_slot->connection, sql.data,
 						   echo, tabname, progname, parallel);
 
-		if (cell)
-			cell = cell->next;
+		cell = cell->next;
 	} while (cell != NULL);
 
 	if (parallel)
@@ -662,9 +686,7 @@ vacuum_all_databases(vacuumingOptions *vacopts,
  */
 static void
 prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
-					   vacuumingOptions *vacopts, const char *table,
-					   bool table_pre_qualified,
-					   const char *progname, bool echo)
+					   vacuumingOptions *vacopts, const char *table)
 {
 	resetPQExpBuffer(sql);
 
@@ -725,14 +747,8 @@ prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
 			appendPQExpBufferStr(sql, " ANALYZE");
 	}
 
-	if (table)
-	{
-		appendPQExpBufferChar(sql, ' ');
-		if (table_pre_qualified)
-			appendPQExpBufferStr(sql, table);
-		else
-			appendQualifiedRelation(sql, table, conn, progname, echo);
-	}
+	appendPQExpBufferChar(sql, ' ');
+	appendPQExpBufferStr(sql, table);
 	appendPQExpBufferChar(sql, ';');
 }
 
-- 
2.16.5