On Fri, Mar 29, 2019 at 11:26 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Fri, Mar 29, 2019 at 4:53 AM Robert Haas <robertmh...@gmail.com> wrote: > > > > On Tue, Mar 26, 2019 at 10:31 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > Thank you for reviewing the patch. > > > > I don't think the approach in v20-0001 is quite right. > > > > if (strcmp(opt->defname, "verbose") == 0) > > - params.options |= VACOPT_VERBOSE; > > + params.options |= defGetBoolean(opt) ? VACOPT_VERBOSE : 0; > > > > It seems to me that it would be better to do declare a separate > > boolean for each flag at the top; e.g. bool verbose. Then here do > > verbose = defGetBoolean(opt). And then after the loop do > > params.options = (verbose ? VACOPT_VERBOSE : 0) | ... similarly for > > other options. > > > > The thing I don't like about the way you have it here is that it's not > > going to work well for options that are true by default but can > > optionally be set to false. In that case, you would need to start > > with the bit set and then clear it, but |= can only set bits, not > > clear them. I went and looked at the VACUUM (INDEX_CLEANUP) patch on > > the other thread and it doesn't have any special handling for that > > case, which makes me suspect that if you use that patch, the reloption > > works as expected but VACUUM (INDEX_CLEANUP false) doesn't actually > > succeed in disabling index cleanup. The structure I suggested above > > would fix that. > > > > You're right, the previous patches are wrong. Attached the updated > version patches. >
These patches conflict with the current HEAD. Attached the updated patches. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
v22-0001-Add-parallel-option-to-VACUUM-command.patch
Description: Binary data
v22-0002-Add-paralell-P-option-to-vacuumdb-command.patch
Description: Binary data