On Tue, Jan 15, 2019 at 6:00 PM Masahiko Sawada <sawada.m...@gmail.com>
wrote:
>
> Rebased.
>
I started reviewing the patch, I didn't finish my review yet.
Following are some of the comments.
+ <term><literal>PARALLEL <replaceable
class="parameter">N</replaceable></literal></term>
+ <listitem>
+ <para>
+ Execute index vacuum and cleanup index in parallel with
I doubt that user can understand the terms index vacuum and cleanup index.
May be it needs some more detailed information.
- VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7 /* don't skip any pages */
+ VACOPT_PARALLEL = 1 << 7, /* do lazy VACUUM in parallel */
+ VACOPT_DISABLE_PAGE_SKIPPING = 1 << 8 /* don't skip any pages */
+} VacuumOptionFlag;
Any specific reason behind not adding it as last member of the enum?
-typedef enum VacuumOption
+typedef enum VacuumOptionFlag
{
I don't find the new name quite good, how about VacuumFlags?
+typedef struct VacuumOption
+{
How about VacuumOptions? Because this structure can contains all the
options provided to vacuum operation.
+ vacopt1->flags |= vacopt2->flags;
+ if (vacopt2->flags == VACOPT_PARALLEL)
+ vacopt1->nworkers = vacopt2->nworkers;
+ pfree(vacopt2);
+ $$ = vacopt1;
+ }
As the above statement indicates the the last parallel number of workers
is considered into the account, can we explain it in docs?
postgres=# vacuum (parallel 2, verbose) tbl;
With verbose, no parallel workers related information is available.
I feel giving that information is required even when it is not parallel
vacuum also.
Regards,
Haribabu Kommi
Fujitsu Australia
