Re: Re: sampling.c and potential divisions by 0 ang log(0) with tablesample and ANALYZE in 9.5

On 2015-06-30 18:21, Tom Lane wrote:
> I wrote:
>> Petr Jelinek <petr(at)2ndquadrant(dot)com> writes:
>>> On 2015-06-25 10:01, Michael Paquier wrote:
>>>> I think that we should change the returned double to be (0.0,1.0]
>
>>> Agreed.
>
>> I find this to be a pretty bad idea. That definition is simply weird;
>> where else in the world will you find a random number generator that does
>> that? What are the odds that any callers are actually designed for that
>> behavior?
>
> And, in fact, a bit of looking quickly finds a counterexample, in
> analyze.c:
>
> int k = (int) (targrows * sampler_random_fract(rstate.randstate));
>
> Assert(k >= 0 && k < targrows);
>
> You can't just whack this around to satisfy some new call sites without
> considering the behavior of existing use-cases.
>

Right, very good point.

So, we used to protect against this problem by using long value and doing:
((double) random() + 1) / ((double) MAX_RANDOM_VALUE + 2)

Maybe best solution is to have pg_lrand48() variant which accepts seed
as parameter same way pg_erand48() does and use the same logic we used
to have before sampling was added.

The attached patch does that and it does not even need changes in the
regression tests output which I consider to be a nice bonus.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services