Subject: Re: [HACKERS] Custom compression methods
To: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>,
 David Steele <david@pgmasters.net>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>,
 Dmitry Dolgov <9erthalion6@gmail.com>,
 PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
References: <20171201194859.le5hvnnrjzhxhm2t@alvherre.pgsql>
 <CAPpHfdtmy_hN_-D9OJ-BwHb_PqUcKWq97kZPtOv6o1g065L8jw@mail.gmail.com>
 <add85c85-0e85-aff5-d7e6-14c99c715251@postgrespro.ru>
 <CAPpHfdtxo7VYd1hrrXAMkBZOk-ZPNhH=SFaJwaGedT5sZ34VrA@mail.gmail.com>
 <03c376ed-839f-35f4-5f03-35b21b47e9a2@postgrespro.ru>
 <CAPpHfduorgpFjQprDPXcUz3MYiau66z+fdrrepxhF+ZfDcy-aA@mail.gmail.com>
 <20180618173045.7f734aca@wp.localdomain>
 <20180702155624.542860bc@wp.localdomain>
 <CAPpHfduytTe0kxfDMZLWY=DXm0GBQKy-Km_gFcgpZ2UKAdHR5Q@mail.gmail.com>
 <20180906182713.502e1107@wp.localdomain>
 <20181101145437.1e00cb99@wp.localdomain>
 <CA+q6zcUTF97=Ao1C2b_0_6znzZX-eQtSB-KvTwZBV_X+e3StJw@mail.gmail.com>
 <20181203154332.7b1f4d6f@wp.localdomain>
 <20190228164415.62bd4107@ildus-work.localdomain>
 <53ab954a-bf44-2bff-fbe0-29f093899604@pgmasters.net>
 <CAPpHfdsQtby_G0HxPMrDA9G=swhC4o0sbm9hZ35A_uP8uw2TEQ@mail.gmail.com>
 <e686c67e-ec70-c081-825d-9a58a0a63595@pgmasters.net>
 <20190315125203.5da43edb@ildus-work.localdomain>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <59ab2e0c-fd31-a063-28f8-9615dfe721ae@2ndquadrant.com>
Date: Mon, 18 Mar 2019 23:08:52 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <20190315125203.5da43edb@ildus-work.localdomain>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 3/15/19 12:52 PM, Ildus Kurbangaliev wrote:
> On Fri, 15 Mar 2019 14:07:14 +0400
> David Steele <david@pgmasters.net> wrote:
> 
>> On 3/7/19 11:50 AM, Alexander Korotkov wrote:
>>> On Thu, Mar 7, 2019 at 10:43 AM David Steele <david@pgmasters.net 
>>> <mailto:david@pgmasters.net>> wrote:
>>>
>>>     On 2/28/19 5:44 PM, Ildus Kurbangaliev wrote:
>>>
>>>      > there are another set of patches.
>>>      > Only rebased to current master.
>>>      >
>>>      > Also I will change status on commitfest to 'Needs review'.
>>>
>>>     This patch has seen periodic rebases but no code review that I
>>> can see since last January 2018.
>>>
>>>     As Andres noted in [1], I think that we need to decide if this
>>> is a feature that we want rather than just continuing to push it
>>> from CF to CF.
>>>
>>>
>>> Yes.  I took a look at code of this patch.  I think it's in pretty
>>> good shape.  But high level review/discussion is required.
>>
>> OK, but I think this patch can only be pushed one more time, maximum, 
>> before it should be rejected.
>>
>> Regards,
> 
> Hi,
> in my opinion this patch is usually skipped not because it is not
> needed, but because of its size. It is not hard to maintain it until
> commiters will have time for it or I will get actual response that
> nobody is going to commit it.
> 

That may be one of the reasons, yes. But there are other reasons, which
I think may be playing a bigger role.

There's one practical issue with how the patch is structured - the docs
and tests are in separate patches towards the end of the patch series,
which makes it impossible to commit the preceding parts. This needs to
change. Otherwise the patch size kills the patch as a whole.

But there's a more important cost/benefit issue, I think. When I look at
patches as a committer, I naturally have to weight how much time I spend
on getting it in (and then dealing with fallout from bugs etc) vs. what
I get in return (measured in benefits for community, users). This patch
is pretty large and complex, so the "costs" are quite high, while the
benefits from the patch itself is the ability to pick between pg_lz and
zlib. Which is not great, and so people tend to pick other patches.

Now, I understand there's a lot of potential benefits further down the
line, like column-level compression (which I think is the main goal
here). But that's not included in the patch, so the gains are somewhat
far in the future.

But hey, I think there are committers working for postgrespro, who might
have the motivation to get this over the line. Of course, assuming that
there are no serious objections to having this functionality or how it's
implemented ... But I don't think that was the case.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services