From 3c2ee6a10ea529a8eacd4cfb8f6754ca708dfa93 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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