Re: sortsupport for text

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: sortsupport for text
Date: 2012-10-08 20:35:37
Message-ID: CA+TgmoZJFHrFXSt+ge0+XtzcdpXbsvSgZ=GgScO+_7EZ_UQyrA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 8, 2012 at 12:26 PM, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> If it was the case that you were only 50% of the way to getting
> something committable, I guess I'd understand; this is, after all, a
> volunteer effort, and you are of course free to pursue or not pursue
> whatever you like. It's more like 95% though, so I have a hard time
> understanding this attitude.
>
> The only criticism that I imagine that you could be referring to is my
> criticism (which was seconded by Tom) concerning the approach to
> sizing a buffer that is used as state between successive calls to the
> sortsupport routine. This was a fairly trivial point even within the
> context of this patch. It was regrettable that it got so out of hand,
> but that certainly wasn't my intention.
>
> I must say that I find this disheartening as a reviewer, particularly
> since it comes from someone who reviews a lot of patches (including
> most of my own), and has often complained about the difficulties that
> reviewers face. Is it really so hard for you to accept a criticism
> from me? I didn't expect you to fully accept my criticism; I only
> expected you to take it into consideration, and possibly let it go for
> the sake of progress.
>
> For what it's worth, I have high regard for your work generally. In
> all sincerity, this appears to me to be a slight, and I find it
> upsetting, both on a personal level, and because it creates a concern
> about being able to work with you in the future, which is certainly
> something that I've benefited from in the past, and hope to continue
> to benefit from.

Hey, if me deciding I don't want to work on a patch any more is going
to make you feel slighted, then you're out of luck. The archives are
littered with people who have decided to stop working on things
because the consensus position on list was different than the approach
that they personally favored, and I have as much right to do that as
anyone else. I think that you are perfectly entitled to feel
disappointed that I don't like your suggestions, even as I am
disappointed that the patch encountered so much resistance. But let's
not turn it into a personal issue. I don't think that you're a bad
person because you don't like my approach, and I hope you don't think
I'm a bad person because I don't like yours. If you do, then that's
just something we're both going to have to live with, because I am not
going to make a policy of working on things because other people will
be offended if I don't.

As to what the substantive issue is, well, yeah, I don't like the
memory allocation strategy that you and Tom are proposing. In the
worst case it chews up a lot of extra memory, and in the best case
there's no measurable performance benefit - or at least nobody done
any work to demonstrate that there is one. I've already pointed out
that, in fact, the strategy of doing power-of-two allocations is
rarely followed for very large allocations, a point to which I believe
no one has responded substantively. Now I don't think anyone else is
required to respond to that point, but neither am I required to revise
the patch into a form that I don't agree with. Equally, I will not
presume to commit it in a form that you and Tom do not agree with.
That would be asinine, and I try (not always successfully) not to be
an ass. It is not as if anyone has phrased this as a maybe-we-should
sort of argument; there have been quite definite arguments on both
sides over apparently strongly-held positions. I had hoped that this
was going to be a quick and non-controversial win, but 74 email
messages later it has become clear that it will be neither of those
things.

I do not have a problem with accepting review feedback from you or
from anyone else. There are many examples in the archives of cases
where I have improved my code based on review feedback; indeed, the
possibility of getting high-quality feedback from the very smart
developers who inhabit this mailing list is for me a principle
attraction of working on PostgreSQL, not to mention a major reason why
PostgreSQL is as good a product as it is. However, that general truth
does not oblige me to accept any particular piece of feedback as
well-founded, and this is hardly the first suggestion that I have
argued against in four years as a PostgreSQL developer, nor the first
of my patches to land on the cutting-room floor. Nor do all of my
suggestions for other people's patches get adopted either.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2012-10-08 20:36:54 Re: why repl_gram.h?
Previous Message Tom Lane 2012-10-08 20:33:29 Re: getopt() and strdup()