On 3/28/23 18:07, gkokola...@pm.me wrote: > > ------- Original Message ------- > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me <gkokola...@pm.me> > wrote: > >> >> ------- Original Message ------- >> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra >> tomas.von...@enterprisedb.com wrote: >> >>> This leaves the empty-data issue (which we have a fix for) and the >>> switch to LZ4F. And then the zstd part. >> >> Please expect promptly a patch for the switch to frames. > > Please find the expected patch attached. Note that the bulk of the > patch is code unification, variable renaming to something more > appropriate, and comment addition. These are changes that are not > strictly necessary to switch to LZ4F. I do believe that are > essential for code hygiene after the switch and they do belong > on the same commit. >
I think the patch is fine, but I'm wondering if the renames shouldn't go a bit further. It removes references to LZ4File struct, but there's a bunch of functions with LZ4File_ prefix. Why not to simply use LZ4_ prefix? We don't have GzipFile either. Sure, it might be a bit confusing because lz4.h uses LZ4_ prefix, but then we probably should not define LZ4_compressor_init ... Also, maybe the comments shouldn't use "File API" when compress_io.c calls that "Compressed stream API". regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company