Re: knngist patch support

On Fri, Feb 12, 2010 at 3:44 PM, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su> wrote:
> This is not fair,Robert. Everything was discussed in -hackers.I assume
> reviewer
> should follow discussion at least, he is a member of our community. Mailing
> list archive was/is/will our the best knowledge base.

Dude, there's no fair or unfair here; I'm just telling you what I
think. There are not six people who follow this mailing list more
closely than I do, and when I started looking at this patch it took me
two hours to figure out why you made those changes to the opclass
machinery. It's not documented anywhere either in the patch or in the
email in which the patch was submitted. That made it hard for me. If
that makes me stupid, then I'm stupid, but then probably there are a
lot of stupid people around here.

> For example, regarding
> changes in the opclass infrastructure you complain, you can see your reply
> to Teodor's message
> http://archives.postgresql.org/message-id/603c8f070912292255u30e1983bi22ed5778bd2ce890@mail.gmail.com
> which contains description of amcanorderbyop flag.

OK, so in one email message that is not the email in which you
submitted the patch there is one sentence of explanation that I failed
to remember six weeks later when I looked at the patch.

> Frankly, I think we see here limit of our resources. Let me explain this.
> We splitted patch by several parts - 2 parts are about contrib modules
> (rather trivial), 1 - is a gist changes, 1 - planner changes and 1 part -
> some proc changes. The most serious are gist and planner patches. We develop
> GiST for many years and know almost everything there and could say that
> we're
> responsible for GiST. I don't know if anybody from -hackers could review our
> patch for planner better than Tom, but he is busy and will be busy.
> So, any serious feature, which touch planner doomed to be rejected because
> of
> lack of reviewer.

I do think resource limitations play a role. For at least as long as
I have been involved in the community, we have relied very heavily on
Tom Lane to review nearly every patch and commit more than half of
them. As our group of developers grows, that is unsustainable, which
I believe to be part of the reason that -core just created four new
committers; as well as part of the reason for the CommitFest process,
which tries to enlist non-committer reviewers. But none of us are Tom
Lane. The part of your patch that took me two hours to figure out
probably would have taken Tom twenty minutes (maybe less). But even
if we assume that I am as smart as Tom Lane and that I will spend as
much time working on PostgreSQL as Tom Lane, both of which may well be
false, I won't know the code base as well as he does now until ~2020.

The only way we can get past that is, first, by splitting up the work
across as many people as possible, and second, by making it as easy as
possible for the reviewer to understand what the code is about. And
at least if the reviewer is me, *documentation helps*.

I'm going to respond to the part of this email that's about moving
this patch forward with a separate email.

...Robert