Hi Dimitri, thanks for reviewing my patch!
On Fri, Nov 19, 2010 at 2:44 PM, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: > I think I'd like to see a separate patch for the new compression > support. Sorry about that, I realize that's extra work… I guess it wouldn't be a very big deal but I also doubt that it makes the review that much easier. Basically the compression refactor patch would just touch pg_backup_custom.c (because this is the place where the libz compression is currently burried into) and the two new compress_io.(c|h) files. Everything else is pretty much the directory stuff and is on top of these changes. > 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? Sure we could. But I wanted to wait with any fancy function pointer stuff until we have decided if we want to include the liblzf support at all. The #ifdefs might be a bit ugly but in case we do not include liblzf support, it's the easiest way to take it out again. As written in my introduction, this patch is not really about liblzf, liblzf is just a proof of concept for factoring out the compression part and I have included it, so that people can use it and see how much speed improvement they get. > 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. Sure! I completely agree... > 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 :) Well, not sure if we can just change the copyright notice, because in the end the structure was copied from one of the other files which all have the copyright notice in them, so my work is based on those other files... > I'm hesitant as far as marking the patch "Waiting on author" to get it > split. Joachim, what do you think? I will see if I can split it. Joachim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
