|From:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>|
|To:||Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: pgbench: Skipping the creating primary keys after initialization|
|Views:||Raw Message | Whole Thread | Download mbox|
Patch applies cleanly, compiles, works for me.
>> At least: "Required to invoke" -> "Require to invoke".
I meant the added one about -I, not the pre-existing one about -i.
>> About the code:
>> is_no_vacuum should be a bool?
> We can change it but I think there is no difference actually. So
> keeping it would be better.
I would like to insist a little bit: the name was declared as an int but
passed to init and used as a bool there before the patch. Conceptually
what is meant is really a bool, and I see no added value bar saving a few
strokes to have an int. ISTM that recent pgbench changes have started
turning old int-for-bool habits into using bool when bool is meant.
initialize_cmds initialization: rather use pg_strdup instead of
-I: pg_free before pg_strdup to avoid a small memory leak?
I'm not sure I would have bothered with sizeof(char), but why not!
I'm still a little bit annoyed by checkCustomCmds: when unhappy, it shows
an error message and return false, which immediatly results in exit(1).
However the pattern elsewhere in pgbench is to show the error and exit
immediatly. I would suggest to simplify by void-ing the function and
exiting instead of returning false.
|Next Message||Fabien COELHO||2017-08-28 08:23:55||Re: psql --batch|
|Previous Message||Craig Ringer||2017-08-28 07:56:41||Re: psql --batch|