Re: refactoring basebackup.c

On Thu, Jan 20, 2022 at 8:00 AM Dipesh Pandit <dipesh(dot)pandit(at)gmail(dot)com> wrote:
> Thanks for the feedback, I have incorporated the suggestions and
> updated a new patch v2.

Cool. I'll do a detailed review later, but I think this is going in a
good direction.

> I tried to add the test coverage for server side gzip compression with
> plain format backup using pg_verifybackup. I have modified the test
> to use a flag specific to plain format. If this flag is set then it takes a
> plain format backup (with server compression enabled) and verifies
> this using pg_verifybackup. I have updated (v2-0002) for the test
> coverage.

Interesting approach. This unfortunately has the effect of making that
test case file look a bit incoherent -- the comment at the top of the
file isn't really accurate any more, for example, and the plain_format
flag does more than just cause us to use -Fp; it also causes us NOT to
use --target server:X. However, that might be something we can figure
out a way to clean up. Alternatively, we could have a new test case
file that is structured like 002_algorithm.pl but looping over
compression methods rather than checksum algorithms, and testing each
one with --server-compress and -Fp. It might be easier to make that
look nice (but I'm not 100% sure).

--
Robert Haas
EDB: http://www.enterprisedb.com