Re: [HACKERS] pgbench: Skipping the creating primary keys afterinitialization - Mailing list pgsql-hackers

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.