On Sun, Jul 27, 2014 at 3:00 AM, Peter Geoghegan <pg@heroku.com> wrote:
> On Thu, Jun 12, 2014 at 2:09 PM, Peter Geoghegan <pg@heroku.com> wrote:
>> Thanks for looking into this.
>
> Is anyone going to look at this?
I'm concerned by the licensing information in hyperloglog.c, which reads:
+ * Portions Copyright (c) 2014, PostgreSQL Global Development Group
+ *
+ * Based on Hideaki Ohno's C++ implementation.
There is no commentary whatever on the license applicable to that
code. It appears to be the MIT license:
https://github.com/hideo55/cpp-HyperLogLog
I don't think we should commit anything that's not clearly under the
PostgreSQL license.
Heikki previously made quite firmly the point that you shouldn't blend
the addition of sortsupport logic in general with the poor man's key
optimization in particular; they should be separate patches. He's
right.
Some other concerns:
1. As I think I mentioned before, I think the term "poor man's key" is
just terrible. It conveys, at least to me, no useful information
about what is really being done. If you called it, say,
"strxfrm-prefix comparison", it would be only a few more letters but a
whole lot more informative to future readers of the code.
2. The need to modify analyze.c and nodeMergeAppend.c to disable this
optimization seems odd, and the comments are uninformative, saying
only that it "isn't feasible", but not why. If it were only analyze.c
I might expect that it required some kind of transaction state that
isn't present there, but that doesn't explain the merge-append case.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
