Re: General purpose hashing func in pgbench

Hello Ildar,

Patch v8 applies cleanly and compiles. Global and local "make check ok".
Doc build ok.

>> For me "random seed" is what is passed to srandom, so the variable
>> should rather be named hash_seed because there could also be a random
>> seed (actually, there is in another parallel patch:-). Moreover, this
>> seed may or may not be random if set, so calling it "random_seed" is
>> not desirable.
>
> My intention was to introduce seed variable which potentially could be
> used in different contexts, not only for hash functions.

Yes, good point. I'd need it for the pseudo-random permutation function.

> I renamed it to 'hash_seed' for now. But what do you think about naming
> it simply 'seed' or choosing some other general name?

ISTM "seed" that is prone to being used for something else in a script.
What about ":default_seed", which says it all?

Some minor suggestions:

In "make_func", I'd assert that nargs is positive in the default branch.

The default seed may be created with just one assignment instead of
repeated |= ops. Or not:-)

In evalStandardFunc, I'd suggest to avoid the ? construction and use an
if and a direct setIntValue in both case, removing the "result"
variable, as done in other branches (eg bitwise operators...). Maybe
something like:

if (func == murmur2)
setIntValue(retval, getHashXXX(val, seed));
else if (...)
...
else
Assert(0);

so that it is structurally ready for other hash functions if needed.

Documentation:

In the table, use official names in the description, and add wikipedia
links, something like:

FNV hash ->
<ulink url="https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function">FNV-1a hash</ulink>

murmur2 hash ->
<ulink url="https://en.wikipedia.org/wiki/MurmurHash">MurmurHash2 hash</ulink>

In the text:

"Hash functions accepts" -> "Hash functions
<literal>hash</literal>, <......> and <....> accept*NO S*"

"... provided hash_seed value is used" => "... provided the value of
<literal>:hash_seed</literal> is used, which is initialized randomly
unless set by the command-line <literal>-D</literal> option."

ISTM that the Automatic Variable table should be in alphabetical order.

--
Fabien.