|From:||Teodor Sigaev <teodor(at)sigaev(dot)ru>|
|To:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Raúl Marín Rodríguez <rmrodriguez(at)carto(dot)com>|
|Cc:||Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: [HACKERS] pgbench more operators & functions|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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 */
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
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
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
|Next Message||Tom Lane||2017-12-14 14:38:39||Re: procedures and plpgsql PERFORM|
|Previous Message||Ali Akbar||2017-12-14 12:18:59||Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL|