Hello Mahahiko-san,
My 0.02€ about the patch & feature, which only reflect my point of view:
Please add a number to patches to avoid ambiguities. This was patch was
the second sent on the thread.
There is no need to have both custom_init & init functions. I'll suggest
to remove function "init", rename "custom_init" as "init", and make the
option default to what is appropriate so that it initialize the schema as
expected, and there is only one initialization mechanism.
I would suggest to make initialization sub options (no-vacuum,
foreign-key...) rely on updating the initialization operations instead of
being maintained separately. Currently "--no-vacuum --custom-init=vacuum"
would skip vacuum, which is quite debatable...
I'm not sure of the "custom-initialize" option name, but why not. I
suggest to remove "is_initialize_suite", and make custom-initialize
require -i as seems logical and upward compatible.
ISTM that there should be short names for the phases. Maybe using only one
letter could simplify the code significantly, dropping commas and list,
eg: "t" for "create_table", "d" for "load_data", "p" for "create_pkey",
"f" for "create_fkey", "v" for "vacuum".
I do not think that allowing a space in the list is a shell-wise idea.
I'm also wondering whether using a list is a good option, because it
implies a large parse function, list management and so on. With the one
letter version, it could be just a string to be scanned char by char for
operations.
Otherwise, at least allow short two letter codes (eg: ct ld pk fk va), in
order to avoid writing a very long thing on the command line, which is
quite a pain:
sh> pgbench
--custom-initialize=create_table,load_data,primary_key,foreign_key,vacuum ...
vs
sh> pgbench -i -I tdpfv ...
Maybe there could be short-hands for usual setups, eg "default" for "tdpv"
or maybe "ct,ld,pk,va", "full" for "tdpfv" or maybe "ct,ld,pk,fk,va"...
Typo in doc "initailize" -> "initialize". Option values should be
presented in their logical execution order, i.e. put vacuum at the end.
Typo in help "initilize" -> "initialize". I would suggest to drop the space in
the
option value in the presentation so that quotes are not needed.
Remove the "no-primary-keys" from the long option array as it has
disappeared. You might consider make "custom-initialize" take the 'I'
short option.
Ranting unrelated to this patch: the automatic aid type switching based on
scale is a bad idea (tm), because when trying to benchmark it means that
changing the scale also changes the schema, and you really do not need
that. ISTM that it should always use INT8.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
