Re: [HACKERS] pgbench more operators & functions

Huh, you are fast. Rebase patch during half an hour.

I haven't objection about patch idea, but I see some gotchas in coding.

1) /* Try to convert variable to numeric form; return false on failure */
static bool
makeVariableValue(Variable *var)

as now, makeVariableValue() convert value of eny type, not only numeric

2) In makeVariableValue():
if (pg_strcasecmp(var->svalue, "null") == 0)
...
else if (pg_strncasecmp(var->svalue, "true", slen)

mixing of pg_strcasecmp and pg_strNcasecmp. And, IMHO, result of
pg_strncasecmp("tru", "true", 1) will be 0. It may be good for 't' of 'f' but
it seems too free grammar to accept 'tr' or 'fa' or even 'o' which actually
not known to be on or off.

3) It seems to me that Variable.has_value could be eliminated and then new
PGBT_NOT_SET is added to PgBenchValueType enum as a first (=0) value. This
allows to shorten code and make it more readable, look
setBoolValue(&var->value, true);
var->has_value = true;
The second line actually doesn't needed. Although I don't insist to fix that.

I actually prefer PGBT_NOT_SET instead of kind of PGBT_UNDEFINED, because I
remember a collision in JavaScript with undefined and null variable.

4) valueTypeName()
it checks all possible PgBenchValue.type but believes that field could contain
some another value. In all other cases it treats as error or assert.

Fabien COELHO wrote:
>
>> Attached v16 fixes those two errors. I regenerated the documentation with the
>> new xml toolchain, and made "check" overall and in pgbench.
>
> Attached v17 is a rebase after the zipfian commit.
>

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/