Re: Bug in reindexdb's error reporting

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> The refactoring bits are fine for HEAD. For back-branches I would
> suggest using the simplest patch of upthread.

Makes sense to me too. The refactoring is mostly to make future
additions easier, so there's not much point for back branches.

> That's perhaps too much generic when it comes to grep in the source
> code, why not appending REINDEX_ to each element?

+1

> We could actually remove this default part, so as we get compiler
> warning when introducing a new element.

Right. Also, I was imagining folding the steps together while
building the commands so that there's just one switch() for that,
along the lines of

const char *verbose_option = verbose ? " (VERBOSE)" : "";
const char *concurrent_option = concurrently ? " CONCURRENTLY" : "";

switch (type)
{
case REINDEX_DATABASE:
appendPQExpBufferStr(&sql, "REINDEX%s DATABASE%s %s",
verbose_option, concurrent_option,
fmtId(PQdb(conn)));
break;
case REINDEX_TABLE:
appendPQExpBufferStr(&sql, "REINDEX%s TABLE%s ",
verbose_option, concurrent_option);
appendQualifiedRelation(&sql, name, conn, progname, echo);
break;
....

It seemed to me that this would be more understandable and flexible
than the way it's being done now, though of course others might see
that differently. I'm not dead set on that, just suggesting it.

regards, tom lane