Re: [HACKERS] pgbench more operators & functions

2017-12-15 14:47 GMT+01:00 Teodor Sigaev <teodor@sigaev.ru>:

> 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.
>>>
>>
>> Yep, but it cannot be called like that because slen ==
>> strlen(var->svalue).
>>
> sorry, mistyped
> pg_strncasecmp("tru", "true", 3) 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.
>>>
>>
>> Yes, it really works like that. I tried to make something clearer than
>> "src/bin/psql/variable.c". Maybe I did not succeed.
>>
> Ok, I see. Current coding accepts truexxx, falsexxx, yesxx, noxxx but
> doesn't accept offxxx and onxxx. Not so consistent as it could be. Also it
> doesn't accept 1 and 0 as psql does, but it's obviously why.
>
> I used "PGBT_NO_VALUE" which seemed clearer otherwise a variable may be
>> set and its value would not "not set" which would look strange.
>>
> Agree
>
>
> Sorry, but I found more notices:
> 1) '~' and unary '-' should be commented, it's not so easy to guess about
> how they actually implemented (-1 XOR value, remember that -1 is
> 0xfffff....)
>
> 2)
> -   | expr '%' expr         { $$ = make_op(yyscanner, "%", $1, $3); }
> +   | expr '%' expr         { $$ = make_op(yyscanner, "mod", $1, $3); }
>
> why is MOD operation renamed? Do I miss something in thread?
>
>
>
> Looking to psql and pgbench scripting implementation, isn't it better to
> integrate lua in psql & pgbench?
>

I don't think - I like Lua language, but it means no small new dependency.
These changes are only few lines and nobody expect building complex
applications in pgbench or psql.

Regards

Pavel

>
> --
> Teodor Sigaev                                   E-mail: teodor@sigaev.ru
>                                                    WWW:
> http://www.sigaev.ru/
>