Re: pg_basebackup, manifests and backends older than ~12

On Mon, Apr 13, 2020 at 6:26 PM Michael Paquier <michael@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