Re: Abbreviated keys for Numeric

On Mon, Mar 23, 2015 at 1:01 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Sun, Mar 22, 2015 at 1:01 PM, Andrew Gierth
> <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>> The substance of the code is unchanged from my original patch. I didn't
>> add diagnostic output to numeric_abbrev_abort, see my separate post
>> about the suggestion of a GUC for that.
>
> I don't think that V2 really changed the substance, which seems to be
> the implication of your remarks here. You disagreed with my decision
> on NULL values - causing me to reconsider my position (so that's now
> irrelevant) - and you disagreed with not including support for 32-bit
> platforms. Those were the only non-stylistic changes, though. I
> certainly didn't change any details of the algorithm that you
> proposed, which, FWIW, I think is rather clever. I added a few
> defensive assertions to the encoding/conversion routine (which I see
> you've removed in V3, a long with a couple of other helpful
> assertions), and restructured and expanded upon the comments, but
> that's all.
>
> You haven't really taken into my account my V2 feedback with this V3
> revision. Even after you yourself specifically called out your
> non-explanation of excess-44 as a possible point of confusion for
> readers of your patch, you didn't change or expand upon your remarks
> on that one iota.

Guys, can we please knock it off with the dueling patches?

Peter, it's really not all that helpful to take somebody else's patch,
rewrite it in a form that they may or may not agree with (even if it's
just the comments), and post that as "v2". And when the person then
posts "v3" that reverts most of your changes, don't go put them all
back and call that "v4". Instead, you should take the hint: these are
not "versions" of the same patch - they are two different approaches
to the same problem. In this type of situation, I generally post my
patch with a name like "topicofthepatch-rmh-v1.patch" or
"topicofthepatch-rmh-20150323.patch", putting my initials in there to
show that this is my version of the patch, not the original author's
and that it may or may not be endorsed by the original author. Having
26 versions of this patch where all of the odd-numbered versions looks
like Andrew's original version and all of the even-numbered versions
look like Peter's "v2" is not going to make anybody happy - not either
of you, not me, and not anybody else here.

The typical style of review here, which I endorse, is to tell the
other person what you think they should change. There is a place for
directly posting a new version yourself, when the amount of cleanup
required is too great to be articulated in an email, and you really
want to get the thing committed; or when the original author has
disappeared and you want to take up the work. But you should try to
do that only in cases where you are fairly sure your work will be
welcome because, quite aside from whether it ruffles any feathers,
it's easy to waste a lot of time rewriting something only to find out
that others don't agree with your rewrites. Discussion helps to avoid
that.

Furthermore, when there *is* a disagreement about something, the thing
to do is to ask for (or simply wait for) the opinions of others, not
dig in your heals. Andrew doesn't have a *right* to have his version
committed, and you don't have a *right* to change it. What we all
have a right to do is discuss, and hopefully agree on, what is best.

Thanks,

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