> On 25 Jun 2025, at 17:58, Tom Lane <t...@sss.pgh.pa.us> wrote: > I looked over this patchset briefly, and found a couple of nits:
Thanks for looking! > v5-0002, in compress_io.h: > > + * Returns true on success and throws error for all error conditions. > > It doesn't return true anymore. Should be more like > > + * Returns nothing. Exits via pg_fatal for all error conditions. Instead of this I changed the write_func signature to return the number of bytes written as size_t. The API is loosely modelled around the libc stream API so going to void seemed less justifiable than size_t, even if the actual returnvalue is fairly useless due to erroring out via pg_fatal. > In LZ4Stream_write: you dropped the bit about > > - errno = (errno) ? errno : ENOSPC; > > but I think that's still necessary: we must assume ENOSPC if fwrite > doesn't set errno. Doh, of course, fixed in the attached. > Other fwrite callers (write_none, Zstd_write) need > this too. v5-0004 has an instance too, in Zstd_close. I did not check > to see if other fwrite calls are OK, but it'd be good to verify > that they all follow the pattern of presetting errno to 0 and then > replacing that with ENOSPC. I went over all the fwrite callsites *in this patchset* and made sure they follow the pattern. There are a number of more fwrite calls in pg_dump (and elsewhere) which might need the same treatment but I left those for a separate patch to keep this focused on the compression API. -- Daniel Gustafsson
v6-0001-Initial-patch-by-Tom-Lane.patch
Description: Binary data
v6-0002-pg_dump-compression-API-write_func.patch
Description: Binary data
v6-0003-pg_dump-compression-API-open_func.patch
Description: Binary data
v6-0004-pg_dump-compression-API-close_func.patch
Description: Binary data
v6-0005-pg_dump-compression-API-LZ4Stream_init.patch
Description: Binary data
v6-0006-pg_dump-compression-API-read_func-gets_func.patch
Description: Binary data
