001: still refers to "gzip", which is correct for -Fp and -Fd but not for -Fc, for which it's more correct to say "zlib". That affects the name of the function, structures, comments, etc. I'm not sure if it's an issue to re-use the basebackup compression routines here. Maybe we should accept "-Zn" for zlib output (-Fc), but reject "gzip:9", which I'm sure some will find confusing, as it does not output. Maybe 001 should be split into a patch to re-use the existing "cfp" interface (which is a clear win), and 002 to re-use the basebackup interfaces for user input and constants, etc.
001 still doesn't compile on freebsd, and 002 doesn't compile on windows. Have you checked test results from cirrusci on your private github account ? 002 says: + save_errno = errno; + errno = save_errno; I suppose that's intended to wrap the preceding library call. 002 breaks "pg_dump -Fc -Z2" because (I think) AllocateCompressor() doesn't store the passed-in compression_spec. 003 still uses <lz4.h> and not "lz4.h". Earlier this year I also suggested to include an 999 patch to change to use LZ4 as the default compression, to exercise the new code under CI. I suggest to re-open the cf patch entry after that passes tests on all platforms and when it's ready for more review. BTW, some of these review comments are the same as what I sent earlier this year. https://www.postgresql.org/message-id/20220326162156.GI28503%40telsasoft.com https://www.postgresql.org/message-id/20220705151328.GQ13040%40telsasoft.com -- Justin