Re: B-Tree support function number 3 (strxfrm() optimization)

On Fri, Aug 1, 2014 at 8:57 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> Robert, Heikki and maybe Alvaro requested and/or explained this split back in
> April. The fact that the would-be first patch was discussed and rejected in
> the past does not counter that request. (I voted against Robert's 2012 patch.
> My opposition was mild even then, and the patch now being a stepping stone to
> a more-compelling benefit certainly tips the scale.) Given that the effect of
> that decision is a moderate procedural change only, even if the request were
> wrong, why continue debating it?

Heikki asked for it, and Robert repeated the request in the past few
days. I am not debating it. I merely don't understand the nature of
the request. I think it's virtually a foregone conclusion that there
should be fmgr elision for text, with very few decisions to be made on
the details of how that would work. The rest of what I have here is
trickier, and is something that must be discussed. Even the
improvement of ~7% that fmgr-elision offers is valuable for such a
common operation.

I don't want to dredge up the details of the 2012 thread, but since
you mention it the fact that the patch was not committed centered on a
silly disagreement on a very fine point, and nothing more. It was
*not* rejected, and my sense at the time was that it was very close to
being committed. I was fairly confident that everyone understood
things around the 2012 patch that way, and I sought clarity on that
point. It's a totally non-surprising and easy to reason about patch.
Clearly at least one person had some reservations about the basic idea
at the time, and it was worth asking what the concern was before
splitting the patch. It is easy to split the patch, but easier still
to answer my question.

> It's fine to incorporate MIT-licensed code; we have many cases of copying
> more-liberally-licensed code into our tree. However, it's important to have
> long-term clarity on the upstream license terms. This is insufficient:
>
>> + /*-------------------------------------------------------------------------
>> + *
>> + * hyperloglog.c
>> + * HyperLogLog cardinality estimator
>> + *
>> + * Portions Copyright (c) 2014, PostgreSQL Global Development Group
>> + *
>> + * Based on Hideaki Ohno's C++ implementation. This is probably not ideally
>> + * suited to estimating the cardinality of very large sets; in particular, we
>
> Usually, the foreign file had a copyright/license notice, and we prepend the
> PostgreSQL notice. See src/port/fls.c for a trivial example.

I will be more careful about this in the future.

--
Peter Geoghegan