On 01/01/2016 12:08 AM, Peter Geoghegan wrote:
> On Thu, Dec 31, 2015 at 12:48 PM, Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> 1) initHyperLogLogError(hyperLogLogState *cState, double error)
>>
>> Instead of specifying bwidth (essentially the number of bits used
>> for addressing in the counter), this allows specifying the expected
>> error rate for the counter, which is
>>
>> error_rate = 1.04 / sqrt(2^bwidth)
>>
>> So for 5% we get bwidth=5, and so on. This makes the API a bit easier
>> the use, because there are pretty much no comments about the meaning
>> of bwidth, and the existing callers simply use 10 without details.
>
> Fair, but you didn't add any better comments!
>
>> 2) freeHyperLogLog(hyperLogLogState *cState)
>>
>> I think it's a good idea to provide function "undoing" what init
>> does, i.e. freeing the internal memory etc. Currently that's trivial
>> to do, but perhaps we'll make the structure more complicated in the
>> future (albeit that might be unlikely).
>
> Seems reasonable.
Attached is v2 of the patch, adding the comments.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services