Re: Proposal : REINDEX xxx VERBOSE

On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:
> Sorry, I forgot attach files.

Review comments:

- Customarily we use int, rather than uint8, for flags variables.  I
think we should do that here also.

- reindex_index() emits a log message either way, but at DEBUG2 level
without VERBOSE and at the INFO level with it.  I think we should skip
it altogether without VERBOSE.  i.e. if (options & REINDEXOPT_VERBOSE)
ereport(...)

- The errmsg() in that function should not end with a period.

- The 000 patch makes a pointless whitespace change to tab-complete.c.

- Instead of an enumerated type (ReindexOption) just use #define to
define the flag value.

Apart from those fairly minor issues, I think this looks pretty solid.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company