Hello,
Patch applies and works.
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.
Hmmm. The existing "is_no_vacuum" variable is typed *both* as int (in
"main") and as bool (in "init"), called by main (yuk!). I see no reason to
choose the bad one for the global:-)
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?
I like it as is, especially as now the associated value is a simple and
short string, I think that it makes sense to have a simple and short
option to trigger it. Moreover -I stands cleanly for "initialization", and
the capital stands for something a little special which it is. Its to good
to miss.
I think that the "-I" it should be added to the "--help" line, as it is
done with other short & long options.
Repeating "-I f" results in multiple foreign key constraints:
Foreign-key constraints:
"pgbench_tellers_bid_fkey" FOREIGN KEY (bid) REFERENCES
pgbench_branches(bid)
"pgbench_tellers_bid_fkey1" FOREIGN KEY (bid) REFERENCES
pgbench_branches(bid)
"pgbench_tellers_bid_fkey2" FOREIGN KEY (bid) REFERENCES
pgbench_branches(bid)
"pgbench_tellers_bid_fkey3" FOREIGN KEY (bid) REFERENCES
pgbench_branches(bid)
I wonder if this could be avoided easily? Maybe by setting the constraint
name explicitely so that the second one fails on the existing one, which
is fine, like for primary keys? Or adding a DROP CONSTRAINT IF EXISTS
before the CREATE CONSTRAINT, like for tables? Or doing nothing about it?
I would prefer the first option.
Maybe the initial cleanup (DROP TABLE) could be made an option added to
the default, so that cleaning up the database could be achieved with some
"pgbench -i -I c", instead of connecting and droping the tables one by one
which I have done quite a few times... What do you think?
Before it is definitely engraved, I'm thinking about the letters:
c - cleanup
t - create table
d - data
p - primary key
f - foreign key
v - vacuum
I think it is mostly okay, but it is the last time to think about it.
Using "d" for cleanup (drop) would mean finding another letter for filling
in data... maybe "g" for data generation? "c" may have been chosen for
"create table", but then would not be available for "cleanup". Thoughts?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
