On Mon, Apr 13, 2020 at 6:26 PM Michael Paquier <mich...@paquier.xyz> wrote: > Well, the documentation tells me that as of protocol.sgml: > "For compatibility with previous releases, the default is > <literal>MANIFEST 'no'</literal>." > > The code also tells me that, in line with the docs: > static void > parse_basebackup_options(List *options, basebackup_options *opt) > [...] > MemSet(opt, 0, sizeof(*opt)); > opt->manifest = MANIFEST_OPTION_NO; > > And there is also a TAP test for that when passing down --no-manifest, > which should not create a backup manifest: > $node->command_ok( > [ > 'pg_basebackup', '-D', "$tempdir/backup2", '--no-manifest', > '--waldir', "$tempdir/xlog2" > ], > > So, it seems to me that it is fine to remove this block, as when > --no-manifest is used, then "manifest" gets set to false, and then it > does not matter if the MANIFEST clause is added or not as we'd just > rely on the default. Keeping the block would matter if you want to > make the code more robust to a change of the default value in the > BASE_BACKUP query though, and its logic is not incorrect either. So, > if you wish to keep it, that's fine by me, but it looks cleaner to me > to remove it and more consistent with the other options like MAX_RATE, > TABLESPACE_MAP, etc.
Oh, hmm. Maybe I'm getting confused with a previous version of the patch that behaved differently. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
