On Thu, Nov 04, 2021 at 05:02:28PM +0000, gkokola...@pm.me wrote:
> Removed an extra condinional check while switching over compression_method.

Indeed.  My rebase was a bit sloppy here.

> because compression_method is the global option exposed to the whereas
> wal_compression_method is the local variable used to figure out what kind of
> file the function is currently working with. This error was existing at least 
> in
> v9-0002 of $subject.

Right.

> I felt that converting it a do {} while () loop instead, will help with
> readability:
> +           do
> +           {
> <snip>
> +           /*
> +            * No need to continue reading the file when the uncompressed_size
> +            * exceeds WalSegSz, even if there are still data left to read.
> +            * However, if uncompressed_size is equal to WalSegSz, it should
> +            * verify that there is no more data to read.
> +            */
> +           } while (r > 0 && uncompressed_size <= WalSegSz);

No objections from me to do that.  This makes the code a bit easier to
follow, indeed.

> I would like to have a bit more test coverage in the case for 
> FindStreamingStart().
> Specifically for the case that a lz4-compressed segment larger than WalSegSz 
> exists.

The same could be said for gzip.  I am not sure that this is worth the
extra I/O and pg_receivewal commands, though.

I have spent an extra couple of hours staring at the code, and the
whole looked fine, so applied.  While on it, I have tested the new TAP
tests with all the possible combinations of --without-zlib and
--with-lz4.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to