Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization

Hello Masahiko-san,

Patch applies cleanly, compiles, works for me.

>> At least: "Required to invoke" -> "Require to invoke".
>
> Fixed.

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 
pg_malloc/strcpy?

-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.

-- 
Fabien.