Re: pgbench: Skipping the creating primary keys after initialization

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
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
Date: 2017-08-29 01:27:50
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

On Mon, Aug 28, 2017 at 4:59 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> Hello Masahiko-san,
> Patch applies cleanly, compiles, works for me.

Thank you for reviewing!

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

Since is_no_vacuum is a existing one, if we follow the habit we should
change other similar variables as well: is_init_mode,
do_vacuum_accounts and debug. And I think we should change them in a
separated patch.

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

Agreed, fixed.

After more thought, I'm bit inclined to not have a short option for
--custom-initialize because this option will be used for not primary
cases. It would be better to save short options for future
enhancements of pgbench. Thought?

Attached latest patch.


Masahiko Sawada
NTT Open Source Software Center

Attachment Content-Type Size
pgbench_custom_initialization_v6.patch application/octet-stream 13.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-08-29 01:30:55 Improving overflow checks when adding tuple to PGresult Re: [GENERAL] Retrieving query results
Previous Message Craig Ringer 2017-08-29 01:26:13 Re: Custom allocators in libpq