Hello Tom,
>> Set random seed for pgbench.
>
> So this patch has ignored the possibility of not having pg_strong_random.
I assumed that pg_strong_random is always available, but may fail with an
error if a strong random source is not available, in which case the error
message should be enough for the user to figure out that they cannot use
the 'rand' value and must rely on 'time' or an integer.
The error message can be improved in this case.
> That's moving the portability goalposts in a way that I do not find
> acceptable for such a marginal feature. What behavior do you think we
> should have for platforms without that --- accept the seed=rand option
> but then error out, or not recognize the option at all?
The first option is currently implemented and documented, and seems fine.
> BTW, I also note that the patch is constructing error messages from
> sentence fragments, which is verboten per translatability policy.
> While that's perhaps not too important as long as we don't have message
> translations for pgbench, it still seems like something to fix now not
> later.
The elements of the structure could be translatable independently: one
part says there is an error, the other provides the context in which this
error was triggered. This could be made a little clearer by using
parentheses for instance. Otherwise to comply with translability policy
the function can be made to return a bool and then the caller show the
context.
In the attached, I improved the error message on 'rand' error and printed
the error context outside of the function.
Thanks for the review.
--
Fabien.
