On Wed, May 13, 2015 at 2:49 PM, Sawada Masahiko <sawada.m...@gmail.com> wrote: > > On Thu, May 14, 2015 at 12:28 AM, Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko <sawada.m...@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. > > > > Thank you for reviwing.. > All fixed. >
IMHO we don't need "pg_rusage_init(&ru0)" if the verbose options is not setted. Maybe change: + + pg_rusage_init(&ru0); to + + if (options & REINDEXOPT_VERBOSE) + pg_rusage_init(&ru0); Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
