From: Dimitri Fontaine <dimitri@2ndQuadrant.fr>
To: Joachim Wieland <joe@mcknight.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: directory archive format for pg_dump
Date: Fri, 19 Nov 2010 20:44:06 +0100
Message-ID: <m28w0pm73t.fsf@2ndQuadrant.fr> (raw)
In-Reply-To: <AANLkTimUELTXwRSQDQNwxik_k1y3YcH1u-9NgHZqpi9e@mail.gmail.com>
References: <AANLkTimUELTXwRSQDQNwxik_k1y3YcH1u-9NgHZqpi9e@mail.gmail.com>
Hi,
Sharing some thoughts after a first round of reviewing, where I only had
time to read the patch itself.
Joachim Wieland <joe@mcknight.de> writes:
> Since the compression is currently all down in the custom format
> backup code,
> the first thing I've done was refactoring the compression functions
> into a
> separate file. While at it, I have added support for liblzf
> compression.
I think I'd like to see a separate patch for the new compression
support. Sorry about that, I realize that's extra work…
And it could be about personal preferences, but the way you added the
liblzf support strikes me at odd, with all those #ifdefs everywhere. Is
it possible to have a specific file for each supported compression
format, then some routing code in src/bin/pg_dump/compress_io.c?
The routing code already exists but then the file is full of #ifdef
sections to define the right supporting function when I think having a
compress_io_zlib and a compress_io_lzf files would be better.
Then there's the bulk of the new dump format feature in the other part
of the patch, namely src/bin/pg_dump/pg_backup_directory.c. You have to
update the copyright in the file header there, at least :)
I'm yet to devote more time on this part of the patch but it seems like
it's rewriting the full support without using the existing bits. That's
something I have to check, didn't have time to read the existing other
archive formats code there.
I'm hesitant as far as marking the patch "Waiting on author" to get it
split. Joachim, what do you think?
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: dimitri@2ndQuadrant.fr, joe@mcknight.de
Subject: Re: directory archive format for pg_dump
In-Reply-To: <m28w0pm73t.fsf@2ndQuadrant.fr>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox