On Wed, Aug 30, 2017 at 3:39 PM, Fabien COELHO <coe...@cri.ensmp.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
pgbench_custom_initialization_v7.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
