On Fri, May 10, 2019 at 5:33 PM Alvaro Herrera <[email protected]> wrote:
>
> On 2019-May-10, Julien Rouhaud wrote:
>
> > On Fri, May 10, 2019 at 4:43 PM Tom Lane <[email protected]> wrote:
>
> > > Patch is good as far as it goes, but I wonder if it'd be smarter to
> > > convert the function's "type" argument from a string to an enum,
> > > and then replace the if/else chains with switches?
> >
> > I've also thought about it. I think the reason why type argument was
> > kept as a string is that reindex_one_database is doing:
> >
> > appendPQExpBufferStr(&sql, type);
> >
> > to avoid an extra switch to append the textual reindex type. I don't
> > have a strong opinion on whether to change that on master or not.
>
> I did have the same thought. It seem clear now that we should do it :-)
> ISTM that the way to fix that problem is to use the proposed enum
> everywhere and turn it into a string when generating the SQL command,
> not before.
ok :) Patch v2 attached.
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index d6f3efd313..93aecfd733 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -16,8 +16,15 @@
#include "fe_utils/string_utils.h"
+typedef enum ReindexType {
+ DATABASE,
+ SCHEMA,
+ TABLE,
+ 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 +248,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, SCHEMA, host, port,
username, prompt_password, progname, echo, verbose, concurrently);
}
}
@@ -252,7 +259,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, INDEX, host, port,
username, prompt_password, progname, echo, verbose, concurrently);
}
}
@@ -262,7 +269,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, TABLE, host, port,
username, prompt_password, progname, echo, verbose, concurrently);
}
}
@@ -272,7 +279,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, DATABASE, host, port,
username, prompt_password, progname, echo, verbose, concurrently);
}
@@ -280,7 +287,7 @@ 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)
@@ -307,33 +314,73 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
if (verbose)
appendPQExpBufferStr(&sql, "(VERBOSE) ");
- appendPQExpBufferStr(&sql, type);
+ switch(type)
+ {
+ case DATABASE:
+ appendPQExpBufferStr(&sql, "DATABASE");
+ break;
+ case SCHEMA:
+ appendPQExpBufferStr(&sql, "SCHEMA");
+ break;
+ case TABLE:
+ appendPQExpBufferStr(&sql, "TABLE");
+ break;
+ case INDEX:
+ appendPQExpBufferStr(&sql, "INDEX");
+ break;
+ default:
+ pg_log_error("Unrecognized reindex type %d", type);
+ exit(1);
+ break;
+ }
+
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)));
+ switch(type)
+ {
+ case TABLE:
+ /* Fall through. */
+ case INDEX:
+ appendQualifiedRelation(&sql, name, conn, progname, echo);
+ break;
+ case SCHEMA:
+ appendPQExpBufferStr(&sql, name);
+ break;
+ case DATABASE:
+ appendPQExpBufferStr(&sql, fmtId(PQdb(conn)));
+ break;
+ default:
+ pg_log_error("Unrecognized reindex type %d", type);
+ exit(1);
+ break;
+ }
appendPQExpBufferChar(&sql, ';');
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));
- if (strcmp(type, "INDEX") == 0)
- pg_log_error("reindexing of index \"%s\" in database \"%s\" failed: %s",
- name, PQdb(conn), PQerrorMessage(conn));
- 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 TABLE:
+ pg_log_error("reindexing of table \"%s\" in database \"%s\" failed: %s",
+ name, PQdb(conn), PQerrorMessage(conn));
+ break;
+ case INDEX:
+ pg_log_error("reindexing of index \"%s\" in database \"%s\" failed: %s",
+ name, PQdb(conn), PQerrorMessage(conn));
+ break;
+ case SCHEMA:
+ pg_log_error("reindexing of schema \"%s\" in database \"%s\" failed: %s",
+ name, PQdb(conn), PQerrorMessage(conn));
+ break;
+ case DATABASE:
+ pg_log_error("reindexing of database \"%s\" failed: %s",
+ PQdb(conn), PQerrorMessage(conn));
+ break;
+ default:
+ pg_log_error("Unrecognized reindex type %d", type);
+ break;
+ }
PQfinish(conn);
exit(1);
}
@@ -374,7 +421,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, DATABASE, host,
port, username, prompt_password,
progname, echo, verbose, concurrently);
}
