On Wed, Dec 28, 2016 at 9:31 PM, Magnus Hagander <magnus@hagander.net> wrote:
> Conditional tests? It probably wouldn't hurt to have them, but that would be
> something more generic (like we'd need something to actually validate it --
> but it would make sense to have a test that, with compression enabled, would
> verify if the uncompressed file turns out to be exactly 16Mb for example).
Looking at if the build is compiled with libz via SQL or using
pg_config is the way to go here. A minimum is doable.
>> A couple of things to be aware of though:
>> - gzopen, gzwrite and gzclose are used to handle the gz files. That's
>> unconsistent with the tar method that is based on mainly deflate() and
>> more low level routines.
>
> But chosen for simplicity, I assume?
Yep. That's quite in-line with the current code.
>> - I have switched the directory method to use a file pointer instead
>> of a file descriptor as gzwrite returns int as the number of
>> uncompressed bytes written.
>
> I don't really follow that reasoning :) Why does the directory method have
> to change to use a filepointer because of that?
The only reason is that write() returns size_t and fwrite returns int,
while gzwrite() returns int. It seems more consistent to use fwrite()
in this case. Or we don't bother about my nitpicking and just cast
stuff.
>> What do you think about this approach? I'll add that to the next CF.
>
> I haven't reviweed the code in detail but yes, I think this approach is the
> right one.
OK, thanks. I'll think about those conditional tests, correct one or
two things in the patch and submit again soon.
--
Michael