Hello Alik,
>> About the maths: As already said, I'm not at ease with a random_zipfian
>> function which does not display a (good) zipfian distribution. At the
>> minimum the documentation should be clear about the approximations
>> implied depending on the parameter value.
>
> I add one more sentence to documentation to emphasize that degree of
> proximity depends on parameter .
> And also I made restriction on
> parameter, now it can be only in range (0; 1)
Hmmm. On second thought, maybe one or the other is enough, either restrict
the parameter to values where the approximation is good, or put out a
clear documentation about when the approximation is not very good, but it
may be still useful even if not precise.
So I would be in favor of expanding the documentation but not restricting
the parameter beyond avoiding value 1.0.
>> [...]
> Now it prints warning message if array overflowed. To print message only
> one time, it uses global flag, which is available for all threads. And
> theoretically message can be printed more than one time. [...]
> So, should I spend time on solving this issue?
No. Just write a comment.
>> If the zipf cache is constant size, there is no point in using dynamic
>> allocation, just declare an array…
>
> Fixed. Does ZIPF_CACHE_SIZE = 15 is ok?
My guess is yes, till someone complains that it is not the case;-)
>> There should be non regression tests somehow. If the "improve pgbench
>> tap test infrastructure" get through, things should be added there.
>
> I will send tests later, as separate patch.
I think that developping a test would be much simpler with the improved
tap test infrastructure, so I would suggest to wait to know the result of
the corresponding patch.
Also, could you recod the patch to CF 2017-09?
https://commitfest.postgresql.org/14/
--
Fabien.
