On Sun, Jul 27, 2014 at 3:00 AM, Peter Geoghegan <p...@heroku.com> wrote: > On Thu, Jun 12, 2014 at 2:09 PM, Peter Geoghegan <p...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
