Hi, As discussed in https://www.postgresql.org/message-id/caobau_yo61rwno3cw6wvywwh7eympuexhkqufb2nfgodunb...@mail.gmail.com, current coding in reindexdb.c is error prone, and reindex_system_catalogs() is also not really required.
I attach two patches to fix both (it could be squashed in a single commit as both are straightforward), for upcoming v13.
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index 7d02a7f54f..64085e79e9 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -21,7 +21,8 @@ typedef enum ReindexType REINDEX_DATABASE, REINDEX_SCHEMA, REINDEX_TABLE, - REINDEX_INDEX + REINDEX_INDEX, + REINDEX_SYSTEM } ReindexType; static void reindex_one_database(const char *name, const char *dbname, @@ -34,11 +35,6 @@ static void reindex_all_databases(const char *maintenance_db, const char *username, enum trivalue prompt_password, const char *progname, bool echo, bool quiet, bool verbose, bool concurrently); -static void reindex_system_catalogs(const char *dbname, - const char *host, const char *port, - const char *username, enum trivalue prompt_password, - const char *progname, bool echo, bool verbose, - bool concurrently); static void help(const char *progname); int @@ -228,8 +224,8 @@ main(int argc, char *argv[]) dbname = get_user_name_or_exit(progname); } - reindex_system_catalogs(dbname, host, port, username, prompt_password, - progname, echo, verbose, concurrently); + reindex_one_database(NULL, dbname, REINDEX_SYSTEM, host, port, + username, prompt_password, progname, echo, verbose, concurrently); } else { @@ -334,6 +330,10 @@ reindex_one_database(const char *name, const char *dbname, ReindexType type, appendQualifiedRelation(&sql, name, conn, progname, echo); appendPQExpBufferStr(&sql, ";"); break; + case REINDEX_SYSTEM: + appendPQExpBuffer(&sql, "REINDEX%s SYSTEM%s %s;", verbose_str, + concurrent_str, fmtId(PQdb(conn))); + break; } if (!executeMaintenanceCommand(conn, sql.data, echo)) @@ -356,6 +356,10 @@ reindex_one_database(const char *name, const char *dbname, ReindexType type, pg_log_error("reindexing of database \"%s\" failed: %s", PQdb(conn), PQerrorMessage(conn)); break; + case REINDEX_SYSTEM: + pg_log_error("reindexing of system catalogs on database \"%s\" failed: %s", + PQdb(conn), PQerrorMessage(conn)); + break; } PQfinish(conn); exit(1); @@ -406,41 +410,6 @@ reindex_all_databases(const char *maintenance_db, PQclear(result); } -static void -reindex_system_catalogs(const char *dbname, const char *host, const char *port, - const char *username, enum trivalue prompt_password, - const char *progname, bool echo, bool verbose, bool concurrently) -{ - PGconn *conn; - PQExpBufferData sql; - - conn = connectDatabase(dbname, host, port, username, prompt_password, - progname, echo, false, false); - - initPQExpBuffer(&sql); - - appendPQExpBuffer(&sql, "REINDEX"); - - if (verbose) - appendPQExpBuffer(&sql, " (VERBOSE)"); - - appendPQExpBufferStr(&sql, " SYSTEM "); - if (concurrently) - appendPQExpBuffer(&sql, "CONCURRENTLY "); - appendPQExpBufferStr(&sql, fmtId(PQdb(conn))); - appendPQExpBufferChar(&sql, ';'); - - if (!executeMaintenanceCommand(conn, sql.data, echo)) - { - pg_log_error("reindexing of system catalogs failed: %s", - PQerrorMessage(conn)); - PQfinish(conn); - exit(1); - } - PQfinish(conn); - termPQExpBuffer(&sql); -} - static void help(const char *progname) {
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index 897ad9a71a..7d02a7f54f 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -16,8 +16,16 @@ #include "fe_utils/string_utils.h" +typedef enum ReindexType +{ + REINDEX_DATABASE, + REINDEX_SCHEMA, + REINDEX_TABLE, + REINDEX_INDEX +} ReindexType; + static void reindex_one_database(const char *name, const char *dbname, - const char *type, const char *host, + ReindexType type, const char *host, const char *port, const char *username, enum trivalue prompt_password, const char *progname, bool echo, bool verbose, bool concurrently); @@ -241,7 +249,7 @@ main(int argc, char *argv[]) for (cell = schemas.head; cell; cell = cell->next) { - reindex_one_database(cell->val, dbname, "SCHEMA", host, port, + reindex_one_database(cell->val, dbname, REINDEX_SCHEMA, host, port, username, prompt_password, progname, echo, verbose, concurrently); } } @@ -252,7 +260,7 @@ main(int argc, char *argv[]) for (cell = indexes.head; cell; cell = cell->next) { - reindex_one_database(cell->val, dbname, "INDEX", host, port, + reindex_one_database(cell->val, dbname, REINDEX_INDEX, host, port, username, prompt_password, progname, echo, verbose, concurrently); } } @@ -262,7 +270,7 @@ main(int argc, char *argv[]) for (cell = tables.head; cell; cell = cell->next) { - reindex_one_database(cell->val, dbname, "TABLE", host, port, + reindex_one_database(cell->val, dbname, REINDEX_TABLE, host, port, username, prompt_password, progname, echo, verbose, concurrently); } } @@ -272,7 +280,7 @@ main(int argc, char *argv[]) * specified */ if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL) - reindex_one_database(NULL, dbname, "DATABASE", host, port, + reindex_one_database(NULL, dbname, REINDEX_DATABASE, host, port, username, prompt_password, progname, echo, verbose, concurrently); } @@ -280,12 +288,14 @@ main(int argc, char *argv[]) } static void -reindex_one_database(const char *name, const char *dbname, const char *type, +reindex_one_database(const char *name, const char *dbname, ReindexType type, const char *host, const char *port, const char *username, enum trivalue prompt_password, const char *progname, bool echo, bool verbose, bool concurrently) { PQExpBufferData sql; + const char *verbose_str = verbose ? " (VERBOSE)" : ""; + const char *concurrent_str = concurrently ? " CONCURRENTLY" : ""; PGconn *conn; @@ -302,38 +312,51 @@ reindex_one_database(const char *name, const char *dbname, const char *type, initPQExpBuffer(&sql); - appendPQExpBufferStr(&sql, "REINDEX "); - - if (verbose) - appendPQExpBufferStr(&sql, "(VERBOSE) "); - - appendPQExpBufferStr(&sql, type); - appendPQExpBufferChar(&sql, ' '); - if (concurrently) - appendPQExpBufferStr(&sql, "CONCURRENTLY "); - if (strcmp(type, "TABLE") == 0 || - strcmp(type, "INDEX") == 0) - appendQualifiedRelation(&sql, name, conn, progname, echo); - else if (strcmp(type, "SCHEMA") == 0) - appendPQExpBufferStr(&sql, name); - else if (strcmp(type, "DATABASE") == 0) - appendPQExpBufferStr(&sql, fmtId(PQdb(conn))); - appendPQExpBufferChar(&sql, ';'); + switch (type) + { + case REINDEX_DATABASE: + appendPQExpBuffer(&sql, "REINDEX%s DATABASE%s %s;", verbose_str, + concurrent_str, fmtId(PQdb(conn))); + break; + case REINDEX_SCHEMA: + appendPQExpBuffer(&sql, "REINDEX%s SCHEMA%s %s;", verbose_str, + concurrent_str, name); + break; + case REINDEX_TABLE: + appendPQExpBuffer(&sql, "REINDEX%s TABLE%s ", verbose_str, + concurrent_str); + appendQualifiedRelation(&sql, name, conn, progname, echo); + appendPQExpBufferStr(&sql, ";"); + break; + case REINDEX_INDEX: + appendPQExpBuffer(&sql, "REINDEX%s INDEX%s ", verbose_str, + concurrent_str); + appendQualifiedRelation(&sql, name, conn, progname, echo); + appendPQExpBufferStr(&sql, ";"); + break; + } if (!executeMaintenanceCommand(conn, sql.data, echo)) { - if (strcmp(type, "TABLE") == 0) - pg_log_error("reindexing of table \"%s\" in database \"%s\" failed: %s", - name, PQdb(conn), PQerrorMessage(conn)); - else if (strcmp(type, "INDEX") == 0) - pg_log_error("reindexing of index \"%s\" in database \"%s\" failed: %s", - name, PQdb(conn), PQerrorMessage(conn)); - else if (strcmp(type, "SCHEMA") == 0) - pg_log_error("reindexing of schema \"%s\" in database \"%s\" failed: %s", - name, PQdb(conn), PQerrorMessage(conn)); - else - pg_log_error("reindexing of database \"%s\" failed: %s", - PQdb(conn), PQerrorMessage(conn)); + switch (type) + { + case REINDEX_TABLE: + pg_log_error("reindexing of table \"%s\" in database \"%s\" failed: %s", + name, PQdb(conn), PQerrorMessage(conn)); + break; + case REINDEX_INDEX: + pg_log_error("reindexing of index \"%s\" in database \"%s\" failed: %s", + name, PQdb(conn), PQerrorMessage(conn)); + break; + case REINDEX_SCHEMA: + pg_log_error("reindexing of schema \"%s\" in database \"%s\" failed: %s", + name, PQdb(conn), PQerrorMessage(conn)); + break; + case REINDEX_DATABASE: + pg_log_error("reindexing of database \"%s\" failed: %s", + PQdb(conn), PQerrorMessage(conn)); + break; + } PQfinish(conn); exit(1); } @@ -374,7 +397,7 @@ reindex_all_databases(const char *maintenance_db, appendPQExpBuffer(&connstr, "dbname="); appendConnStrVal(&connstr, dbname); - reindex_one_database(NULL, connstr.data, "DATABASE", host, + reindex_one_database(NULL, connstr.data, REINDEX_DATABASE, host, port, username, prompt_password, progname, echo, verbose, concurrently); }