| From: | José Arthur Benetasso Villanova <jose(dot)arthur(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: directory archive format for pg_dump |
| Date: | 2010-11-19 21:28:03 |
| Message-ID: | AANLkTi=AEi3F6jC0Zy3dG9njiwJJC8Y6V090-hMonyyn@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Dimitri and Joachim.
I've looked the patch too, and I want to share some thoughts too. I've
used http://wiki.postgresql.org/wiki/Reviewing_a_Patch to guide my
review.
Submission review:
I've apllied and compiled the patch successfully using the current master.
Usability review:
The dir format generated in my database 60 files, with different
sizes, and it looks very confusing. Is it possible to use the same
trick as pigz and pbzip2, creating a concatenated file of streams?
Feature test:
Just a partial review. I can dump / restore using lzf, but didnt
stress it hard to check robustness.
Performance review:
Didnt test it hard too, but looks ok.
Coding review:
Just a shallow review here.
>> I think I'd like to see a separate patch for the new compression
>> support. Sorry about that, I realize that's extra work…
Same feeling here, this is the 1st thing that I notice.
The md5.c and kwlookup.c reuse using a link doesn't look nice either.
This way you need to compile twice, among others things, but I think
that its temporary, right?
--
José Arthur Benetasso Villanova
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Smith | 2010-11-19 21:28:05 | Re: Changes to Linux OOM killer in 2.6.36 |
| Previous Message | Andres Freund | 2010-11-19 21:16:19 | Re: [PATCH] Custom code int(32|64) => text conversions out of performance reasons |