On 2013-06-19 00:15:56 -0700, Hitoshi Harada wrote:
> On Wed, Jun 5, 2013 at 8:01 AM, Andres Freund <andres@2ndquadrant.com>wrote:
> >
> > Two patches attached:
> > 1) add snappy to src/common. The integration needs some more work.
> > 2) Combined patch that adds indirect tuple and snappy compression. Those
> > coul be separated, but this is an experiment so far...
> >
> >
> >
> I took a look at them a little. This proposal is a super set of patch
> #1127.
> https://commitfest.postgresql.org/action/patch_view?id=1127
>
> - <endian.h> is not found in my mac. Commented it out, it builds clean.
> - I don't see what the added is_inline flag means in
> toast_compress_datum(). Obviously not used, but I wonder what was the
> intention.
Hm. I don't think you've looked at the latest version of the patch,
check
http://archives.postgresql.org/message-id/20130614230625.GD19641@awork2.anarazel.de
- that should be linked from the commitfest. The is_inline part should
be gone there.
> - By this,
> * compression method. We could just use the two bytes to store 3 other
> * compression methods but maybe we better don't paint ourselves in a
> * corner again.
> you mean two bits, not two bytes?
Yes, typo... The plan is to use those two bits in the following way
- 00 pglz
- 01 snappy/lz4/whatever
- 10 another
- 11 one extra byte
> And patch adds snappy-c in src/common. I definitely like the idea to have
> pluggability for different compression algorithm for datum, but I am not
> sure if this location is a good place to add it. Maybe we want a modern
> algorithm other than pglz for different components across the system in
> core, and it's better to let users choose which to add more. The mapping
> between the index number and compression algorithm should be consistent for
> the entire life of database, so it should be defined at initdb time. From
> core maintainability perspective and binary size of postgres, I don't think
> we want to put dozenes of different algorithms into core in the future.
> And maybe someone will want to try BSD-incompatible algorithm privately.
We've argued about this in the linked thread and I still think we should
add one algorithm now, and when that one is outdated - which probably
will be some time - replace it. Building enough infrastructure to make
this really pluggable is not likely enough to be beneficial to many.
There will be a newer version of the patch coming today or tomorrow, so
there's probably no point in looking at the one linked above before
that...
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services