Re: pgbench: Skipping the creating primary keys after initialization

On Wed, Aug 30, 2017 at 3:39 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> Hello,
>
>>> 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:-)
>>
>>
>> Yeah, I think this might be a good timing to re-consider int-for-bool
>> habits in pgbench. If we decided to change is_no_vacuum to bool I want
>> to change other similar variables as well.
>
>
> Franckly I would be fine with that, but committers might get touchy about
> "unrelated changes" in the patch... The "is_no_vacuum" is related to the
> patch and is already a bool -- if you chose the "init" definition as a
> reference -- so it is okay to bool it.

Okay, I changed only is_no_vacuum in this patch and other similar
variables would be changed in another patch.

>
>>> I think that the "-I" it should be added to the "--help" line, as it is
>>> done
>>> with other short & long options.
>>
>>
>> Okay, I'll leave it as of now. Maybe we can discuss later.
>
>
> Maybe we did not understand one another. I'm just suggesting to insert
> -I in the help line, that is change:
>
> " --custom-initialize=[...]+\n"
>
> to
>
> " -I, --custom-initialize=[...]+\n"

Fixed.

> I'm not sure it deserves to be discussed in depth later:-)

Sorry, I meant about having short option --custom-initialize.

>
>>> 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? [...]
>>
>>
>> Good point, I agree with first option.
>
>
> Ok.
>
>>> 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?
>>
>>
>> Yeah, I sometimes wanted that. Having the cleaning up tables option
>> would be good idea.
>
>
> Ok.
>
>> I'd say "g" for data generation would be better. Also, I'm inclined to
>> add a command for the unlogged tables. How about this?
>>
>> c - [c]leanup / or [d]rop tables
>> t - create table / [t]able creation or [c]reate table
>> u - create unlogged table
>> g - data generation / [g]enerate data
>> p - [p]rimary key
>> f - [f]oreign key
>> v - [v]acuum
>
>
> I'm okay with that. I also put an alternative with d/c above, without any
> preference from my part.

Personally I prefer "t" for table creation because "c" for create is a
generic word. We might want to have another initialization command
that creates something.

>
> I'm not sure about "u", though. Unlogged, like tablespace, is an orthogonal
> option: other table creation options (I intend to submit one which conforms
> to the TPC-B standard, that is use an INT8 balance as INT4 is not wide
> enough per spec, and always use an INT8 aid) may be also unlogged or
> tablespaced. So that would mean having two ways to trigger them... thus I
> would avoid it and keep only --unlogged.
>

Yeah, I think I had misunderstood it. -I option is for specifying some
particular initialization steps. So we don't need to have a command as
a option for other initializatoin commands.

Attached latest patch. Please review it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center